From: Joakim Zhang <qiangqing.zhang@nxp.com> To: Florian Fainelli <f.fainelli@gmail.com>, "davem@davemloft.net" <davem@davemloft.net>, "kuba@kernel.org" <kuba@kernel.org>, "robh+dt@kernel.org" <robh+dt@kernel.org>, "shawnguo@kernel.org" <shawnguo@kernel.org>, "s.hauer@pengutronix.de" <s.hauer@pengutronix.de>, "festevam@gmail.com" <festevam@gmail.com>, "andrew@lunn.ch" <andrew@lunn.ch> Cc: "kernel@pengutronix.de" <kernel@pengutronix.de>, dl-linux-imx <linux-imx@nxp.com>, "netdev@vger.kernel.org" <netdev@vger.kernel.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Subject: RE: [PATCH net-next 1/3] dt-bindings: net: fsl, fec: add "fsl, wakeup-irq" property Date: Wed, 11 Aug 2021 08:06:57 +0000 [thread overview] Message-ID: <VI1PR04MB6800F8425473916A82F050F5E6F89@VI1PR04MB6800.eurprd04.prod.outlook.com> (raw) In-Reply-To: <90af8051-512d-1230-72a7-8bbcee984939@gmail.com> > -----Original Message----- > From: Florian Fainelli <f.fainelli@gmail.com> > Sent: 2021年8月11日 1:45 > To: Joakim Zhang <qiangqing.zhang@nxp.com>; davem@davemloft.net; > kuba@kernel.org; robh+dt@kernel.org; shawnguo@kernel.org; > s.hauer@pengutronix.de; festevam@gmail.com; andrew@lunn.ch > Cc: kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>; > netdev@vger.kernel.org; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH net-next 1/3] dt-bindings: net: fsl, fec: add "fsl, > wakeup-irq" property > > > > On 8/9/2021 7:21 PM, Joakim Zhang wrote: > > > > Hi Florian, > > > >> -----Original Message----- > >> From: Florian Fainelli <f.fainelli@gmail.com> > >> Sent: 2021年8月10日 2:40 > >> To: Joakim Zhang <qiangqing.zhang@nxp.com>; davem@davemloft.net; > >> kuba@kernel.org; robh+dt@kernel.org; shawnguo@kernel.org; > >> s.hauer@pengutronix.de; festevam@gmail.com; andrew@lunn.ch > >> Cc: kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>; > >> netdev@vger.kernel.org; devicetree@vger.kernel.org; > >> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org > >> Subject: Re: [PATCH net-next 1/3] dt-bindings: net: fsl, fec: add > >> "fsl, wakeup-irq" property > >> > >> > >> > >> On 8/8/2021 10:08 PM, Joakim Zhang wrote: > >>> > >>> Hi Florian, > >>> > >>>> -----Original Message----- > >>>> From: Florian Fainelli <f.fainelli@gmail.com> > >>>> Sent: 2021年8月5日 17:18 > >>>> To: Joakim Zhang <qiangqing.zhang@nxp.com>; davem@davemloft.net; > >>>> kuba@kernel.org; robh+dt@kernel.org; shawnguo@kernel.org; > >>>> s.hauer@pengutronix.de; festevam@gmail.com; andrew@lunn.ch > >>>> Cc: kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>; > >>>> netdev@vger.kernel.org; devicetree@vger.kernel.org; > >>>> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org > >>>> Subject: Re: [PATCH net-next 1/3] dt-bindings: net: fsl, fec: add > >>>> "fsl, wakeup-irq" property > >>>> > >>>> > >>>> > >>>> On 8/5/2021 12:46 AM, Joakim Zhang wrote: > >>>>> Add "fsl,wakeup-irq" property for FEC controller to select wakeup > >>>>> irq source. > >>>>> > >>>>> Signed-off-by: Fugang Duan <fugang.duan@nxp.com> > >>>>> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> > >>>> > >>>> Why are not you making use of the standard interrupts-extended > >>>> property which allows different interrupt lines to be originating > >>>> from different interrupt controllers, e.g.: > >>>> > >>>> interrupts-extended = <&gic GIC_SPI 112 4>, <&wakeup_intc 0>; > >>> > >>> Thanks. > >>> > >>> AFAIK, interrupts-extended should be used instead of interrupts when > >>> a device is connected to multiple interrupt controllers as it > >>> encodes a parent phandle with each interrupt specifier. However, for > >>> FEC controller, all > >> interrupt lines are originating from the same interrupt controllers. > >> > >> OK, so why this custom property then? > >> > >>> > >>> 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. > >>> 4) Now driver treat int0 as the wakeup source by default, it is > >>> broken for > >> i.MX8M. > >> > >> I don't really know what to make of your response, it seems to me > >> that you are carrying some legacy Device Tree properties that were > >> invented when interrupts-extended did not exist and we did not know any > better. > > > > As I described in former mail, it is not related to interrupts-extended > property. > > > > Let's take a look, e.g. > > > > 1) arch/arm/boot/dts/imx7d.dtsi > > interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>; > > interrupt-names = "int0", "int1", "int2", "pps"; > > > > For these 4 interrupts are originating from GIC interrupt controller, > > "int0" for queue 0 and other interrupt signals, containing wakeup; "int1" for > queue 1; "int2" for queue 2. > > > > 2) arch/arm64/boot/dts/freescale/imx8mq.dtsi > > interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>; > > interrupt-names = "int0", "int1", "int2", "pps"; > > > > For these 4 interrupts are also originating from GIC interrupt > > controller, "int0" for queue 0; "int1" for queue 1; "int2" for queue 2 and other > interrupt signals, containing wakeup. > > > > If we want to use WoL feature, we need invoke enable_irq_wake() to let > > this specific interrupt line be a wakeup source. For FEC driver now, > > it treats "int0" as wakeup interrupt by default. Obviously it's not fine for > i.MX8M serials, since SoC mix wakeup interrupt signal into "int2", so I add this > "fsl,wakeup-irq" custom property to indicate which interrupt line contains > wakeup signal. > > > > Not sure if I have explained it clearly enough, from my point of view, I think > interrupts-extended property can't fix this issue, right? > > This is clearer, and indeed interrupts-extended won't fix that, however it seems > to me that this is a problem that ought to be fixed at the interrupt > controller/irq_chip level which should know and be told which interrupt lines > can be made wake-up interrupts or not. From there on, the driver can test with > enable_irq_wake() whether this has a chance of working or not. How can we test with enable_irq_wake()? I agree that interrupt controller can recognize wakeup interrupt is better. > It seems to me that the 'fsl,wakeup-irq' property ought to be within the > interrupt controller Device Tree node (where it would be easier to validate that > the specific interrupt line is correct) as opposed to within the consumer (FEC) > Device Tree node. Not quite understand, could you explain more? > > > > If there is any common properties can be used for it, please let me know. Or > any other better solutions also be appreciated. Thanks. > > There is a standard 'wakeup-source' boolean property that can be added > to any Device Tree node to indicate it can be a wake-up source, but what > you need here is a bitmask, so introducing a custom property may be > appropriate here. Yes, I know 'wakeup-source' boolean property that identify this DEVICE can be a wakeup source, it's not related to interrupt. Best Regards, Joakim Zhang > -- > Florian
WARNING: multiple messages have this Message-ID (diff)
From: Joakim Zhang <qiangqing.zhang@nxp.com> To: Florian Fainelli <f.fainelli@gmail.com>, "davem@davemloft.net" <davem@davemloft.net>, "kuba@kernel.org" <kuba@kernel.org>, "robh+dt@kernel.org" <robh+dt@kernel.org>, "shawnguo@kernel.org" <shawnguo@kernel.org>, "s.hauer@pengutronix.de" <s.hauer@pengutronix.de>, "festevam@gmail.com" <festevam@gmail.com>, "andrew@lunn.ch" <andrew@lunn.ch> Cc: "kernel@pengutronix.de" <kernel@pengutronix.de>, dl-linux-imx <linux-imx@nxp.com>, "netdev@vger.kernel.org" <netdev@vger.kernel.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Subject: RE: [PATCH net-next 1/3] dt-bindings: net: fsl, fec: add "fsl, wakeup-irq" property Date: Wed, 11 Aug 2021 08:06:57 +0000 [thread overview] Message-ID: <VI1PR04MB6800F8425473916A82F050F5E6F89@VI1PR04MB6800.eurprd04.prod.outlook.com> (raw) In-Reply-To: <90af8051-512d-1230-72a7-8bbcee984939@gmail.com> > -----Original Message----- > From: Florian Fainelli <f.fainelli@gmail.com> > Sent: 2021年8月11日 1:45 > To: Joakim Zhang <qiangqing.zhang@nxp.com>; davem@davemloft.net; > kuba@kernel.org; robh+dt@kernel.org; shawnguo@kernel.org; > s.hauer@pengutronix.de; festevam@gmail.com; andrew@lunn.ch > Cc: kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>; > netdev@vger.kernel.org; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH net-next 1/3] dt-bindings: net: fsl, fec: add "fsl, > wakeup-irq" property > > > > On 8/9/2021 7:21 PM, Joakim Zhang wrote: > > > > Hi Florian, > > > >> -----Original Message----- > >> From: Florian Fainelli <f.fainelli@gmail.com> > >> Sent: 2021年8月10日 2:40 > >> To: Joakim Zhang <qiangqing.zhang@nxp.com>; davem@davemloft.net; > >> kuba@kernel.org; robh+dt@kernel.org; shawnguo@kernel.org; > >> s.hauer@pengutronix.de; festevam@gmail.com; andrew@lunn.ch > >> Cc: kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>; > >> netdev@vger.kernel.org; devicetree@vger.kernel.org; > >> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org > >> Subject: Re: [PATCH net-next 1/3] dt-bindings: net: fsl, fec: add > >> "fsl, wakeup-irq" property > >> > >> > >> > >> On 8/8/2021 10:08 PM, Joakim Zhang wrote: > >>> > >>> Hi Florian, > >>> > >>>> -----Original Message----- > >>>> From: Florian Fainelli <f.fainelli@gmail.com> > >>>> Sent: 2021年8月5日 17:18 > >>>> To: Joakim Zhang <qiangqing.zhang@nxp.com>; davem@davemloft.net; > >>>> kuba@kernel.org; robh+dt@kernel.org; shawnguo@kernel.org; > >>>> s.hauer@pengutronix.de; festevam@gmail.com; andrew@lunn.ch > >>>> Cc: kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>; > >>>> netdev@vger.kernel.org; devicetree@vger.kernel.org; > >>>> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org > >>>> Subject: Re: [PATCH net-next 1/3] dt-bindings: net: fsl, fec: add > >>>> "fsl, wakeup-irq" property > >>>> > >>>> > >>>> > >>>> On 8/5/2021 12:46 AM, Joakim Zhang wrote: > >>>>> Add "fsl,wakeup-irq" property for FEC controller to select wakeup > >>>>> irq source. > >>>>> > >>>>> Signed-off-by: Fugang Duan <fugang.duan@nxp.com> > >>>>> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> > >>>> > >>>> Why are not you making use of the standard interrupts-extended > >>>> property which allows different interrupt lines to be originating > >>>> from different interrupt controllers, e.g.: > >>>> > >>>> interrupts-extended = <&gic GIC_SPI 112 4>, <&wakeup_intc 0>; > >>> > >>> Thanks. > >>> > >>> AFAIK, interrupts-extended should be used instead of interrupts when > >>> a device is connected to multiple interrupt controllers as it > >>> encodes a parent phandle with each interrupt specifier. However, for > >>> FEC controller, all > >> interrupt lines are originating from the same interrupt controllers. > >> > >> OK, so why this custom property then? > >> > >>> > >>> 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. > >>> 4) Now driver treat int0 as the wakeup source by default, it is > >>> broken for > >> i.MX8M. > >> > >> I don't really know what to make of your response, it seems to me > >> that you are carrying some legacy Device Tree properties that were > >> invented when interrupts-extended did not exist and we did not know any > better. > > > > As I described in former mail, it is not related to interrupts-extended > property. > > > > Let's take a look, e.g. > > > > 1) arch/arm/boot/dts/imx7d.dtsi > > interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>; > > interrupt-names = "int0", "int1", "int2", "pps"; > > > > For these 4 interrupts are originating from GIC interrupt controller, > > "int0" for queue 0 and other interrupt signals, containing wakeup; "int1" for > queue 1; "int2" for queue 2. > > > > 2) arch/arm64/boot/dts/freescale/imx8mq.dtsi > > interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>; > > interrupt-names = "int0", "int1", "int2", "pps"; > > > > For these 4 interrupts are also originating from GIC interrupt > > controller, "int0" for queue 0; "int1" for queue 1; "int2" for queue 2 and other > interrupt signals, containing wakeup. > > > > If we want to use WoL feature, we need invoke enable_irq_wake() to let > > this specific interrupt line be a wakeup source. For FEC driver now, > > it treats "int0" as wakeup interrupt by default. Obviously it's not fine for > i.MX8M serials, since SoC mix wakeup interrupt signal into "int2", so I add this > "fsl,wakeup-irq" custom property to indicate which interrupt line contains > wakeup signal. > > > > Not sure if I have explained it clearly enough, from my point of view, I think > interrupts-extended property can't fix this issue, right? > > This is clearer, and indeed interrupts-extended won't fix that, however it seems > to me that this is a problem that ought to be fixed at the interrupt > controller/irq_chip level which should know and be told which interrupt lines > can be made wake-up interrupts or not. From there on, the driver can test with > enable_irq_wake() whether this has a chance of working or not. How can we test with enable_irq_wake()? I agree that interrupt controller can recognize wakeup interrupt is better. > It seems to me that the 'fsl,wakeup-irq' property ought to be within the > interrupt controller Device Tree node (where it would be easier to validate that > the specific interrupt line is correct) as opposed to within the consumer (FEC) > Device Tree node. Not quite understand, could you explain more? > > > > If there is any common properties can be used for it, please let me know. Or > any other better solutions also be appreciated. Thanks. > > There is a standard 'wakeup-source' boolean property that can be added > to any Device Tree node to indicate it can be a wake-up source, but what > you need here is a bitmask, so introducing a custom property may be > appropriate here. Yes, I know 'wakeup-source' boolean property that identify this DEVICE can be a wakeup source, it's not related to interrupt. Best Regards, Joakim Zhang > -- > Florian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-08-11 8:08 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-08-05 7:46 [PATCH net-next 0/3] net: fec: add support to select wakeup irq source Joakim Zhang 2021-08-05 7:46 ` Joakim Zhang 2021-08-05 7:46 ` [PATCH net-next 1/3] dt-bindings: net: fsl,fec: add "fsl,wakeup-irq" property Joakim Zhang 2021-08-05 7:46 ` [PATCH net-next 1/3] dt-bindings: net: fsl, fec: add "fsl, wakeup-irq" property Joakim Zhang 2021-08-05 9:18 ` Florian Fainelli 2021-08-05 9:18 ` Florian Fainelli 2021-08-09 5:08 ` Joakim Zhang 2021-08-09 5:08 ` Joakim Zhang 2021-08-09 18:39 ` Florian Fainelli 2021-08-09 18:39 ` Florian Fainelli 2021-08-10 2:21 ` Joakim Zhang 2021-08-10 2:21 ` Joakim Zhang 2021-08-10 14:32 ` Andrew Lunn 2021-08-10 14:32 ` Andrew Lunn 2021-08-11 7:57 ` Joakim Zhang 2021-08-11 7:57 ` Joakim Zhang 2021-08-11 16:16 ` Andrew Lunn 2021-08-11 16:16 ` Andrew Lunn 2021-08-13 18:22 ` Rob Herring 2021-08-13 18:22 ` Rob Herring 2021-08-16 6:48 ` Joakim Zhang 2021-08-16 6:48 ` Joakim Zhang 2021-08-10 17:45 ` Florian Fainelli 2021-08-10 17:45 ` Florian Fainelli 2021-08-11 8:06 ` Joakim Zhang [this message] 2021-08-11 8:06 ` Joakim Zhang 2021-08-12 9:46 ` Florian Fainelli 2021-08-12 9:46 ` Florian Fainelli 2021-08-05 7:46 ` [PATCH net-next 2/3] net: fec: add support to select wakeup irq source Joakim Zhang 2021-08-05 7:46 ` Joakim Zhang 2021-08-05 7:46 ` [PATCH net-next 3/3] arm64: dts: imx8m: add "fsl,wakeup-irq" property for FEC Joakim Zhang 2021-08-05 7:46 ` [PATCH net-next 3/3] arm64: dts: imx8m: add "fsl, wakeup-irq" " Joakim Zhang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=VI1PR04MB6800F8425473916A82F050F5E6F89@VI1PR04MB6800.eurprd04.prod.outlook.com \ --to=qiangqing.zhang@nxp.com \ --cc=andrew@lunn.ch \ --cc=davem@davemloft.net \ --cc=devicetree@vger.kernel.org \ --cc=f.fainelli@gmail.com \ --cc=festevam@gmail.com \ --cc=kernel@pengutronix.de \ --cc=kuba@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-imx@nxp.com \ --cc=linux-kernel@vger.kernel.org \ --cc=netdev@vger.kernel.org \ --cc=robh+dt@kernel.org \ --cc=s.hauer@pengutronix.de \ --cc=shawnguo@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.