| Message ID | 1547797668-30342-2-git-send-email-aisheng.dong@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | irq: imx-irqsteer: add 32 interrupts chan and multi outputs support | expand |
Am Freitag, den 18.01.2019, 07:53 +0000 schrieb Aisheng Dong: > Not all 64 interrupts may be used in one group. e.g. most irqsteer in > imx8qxp and imx8qm subsystems supports only 32 interrupts. > > As the IP integration parameters are Channel number and interrupts number, > let's use fsl,irqs-per-chan to represents how many interrupts supported > by this irqsteer channel. Sorry, but total NACK. I've got to great lengths with dumping the actually implemented register layout on i.MX8M and AFAICS the IRQs are always managed in groups of 64 IRQs, even if less than that are connected as input IRQs. This is what the actually present register set on i.MX8M tells us. Regards, Lucas > Cc: Marc Zyngier <marc.zyngier@arm.com> > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Lucas Stach <l.stach@pengutronix.de> > > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: devicetree@vger.kernel.org > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > --- > .../devicetree/bindings/interrupt-controller/fsl,irqsteer.txt | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt b/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt > index 45790ce..eaabcda 100644 > --- a/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt > +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt > @@ -16,8 +16,8 @@ Required properties: > - #interrupt-cells: Specifies the number of cells needed to encode an > interrupt source. The value must be 1. > - fsl,channel: The output channel that all input IRQs should be steered into. > -- fsl,irq-groups: Number of IRQ groups managed by this controller instance. > - Each group manages 64 input interrupts. > +- fsl,irqs-per-chan: Number of input interrupts per channel. Should be multiple of 32 > + input interrupts and up to 512 interrupts. > > Example: > > @@ -28,7 +28,7 @@ Example: > > clocks = <&clk IMX8MQ_CLK_DISP_APB_ROOT>; > > clock-names = "ipg"; > > fsl,channel = <0>; > > - fsl,irq-groups = <1>; > > + fsl,irqs-per-chan= <64>; > > interrupt-controller; > > #interrupt-cells = <1>; > > };
On 18/01/2019 08:48, Lucas Stach wrote: > Am Freitag, den 18.01.2019, 07:53 +0000 schrieb Aisheng Dong: >> Not all 64 interrupts may be used in one group. e.g. most irqsteer in >> imx8qxp and imx8qm subsystems supports only 32 interrupts. >> >> As the IP integration parameters are Channel number and interrupts number, >> let's use fsl,irqs-per-chan to represents how many interrupts supported >> by this irqsteer channel. > > Sorry, but total NACK. I've got to great lengths with dumping the > actually implemented register layout on i.MX8M and AFAICS the IRQs are > always managed in groups of 64 IRQs, even if less than that are > connected as input IRQs. This is what the actually present register set > on i.MX8M tells us. Also, I'd really like the DT bindings not to change at every release. So whatever change (if any) has to be done for this driver to support existing HW, please make sure that the DT bindings are kept as stable as possible. Thanks, M.
> -----Original Message----- > From: Lucas Stach [mailto:l.stach@pengutronix.de] > Sent: Friday, January 18, 2019 4:48 PM > > Am Freitag, den 18.01.2019, 07:53 +0000 schrieb Aisheng Dong: > > Not all 64 interrupts may be used in one group. e.g. most irqsteer in > > imx8qxp and imx8qm subsystems supports only 32 interrupts. > > > > As the IP integration parameters are Channel number and interrupts > > number, let's use fsl,irqs-per-chan to represents how many interrupts > > supported by this irqsteer channel. > > Sorry, but total NACK. I've got to great lengths with dumping the actually > implemented register layout on i.MX8M and AFAICS the IRQs are always > managed in groups of 64 IRQs, even if less than that are connected as input > IRQs. This is what the actually present register set on i.MX8M tells us. > The register layout varies depends on the irq per channel supports. It's true 64 IRQs on MX8M, but not true for QXP and QM. That's why we shouldn't use fsl,irq-groups. Regards Dong Aisheng > Regards, > Lucas > > > Cc: Marc Zyngier <marc.zyngier@arm.com> > > > Cc: Rob Herring <robh+dt@kernel.org> > > > Cc: Lucas Stach <l.stach@pengutronix.de> > > > Cc: Shawn Guo <shawnguo@kernel.org> > > Cc: devicetree@vger.kernel.org > > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > > --- > > .../devicetree/bindings/interrupt-controller/fsl,irqsteer.txt | > > 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git > > a/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer. > > txt > > b/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer. > > txt > > index 45790ce..eaabcda 100644 > > --- > > a/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer. > > txt > > +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,irqst > > +++ eer.txt > > @@ -16,8 +16,8 @@ Required properties: > > - #interrupt-cells: Specifies the number of cells needed to encode an > > interrupt source. The value must be 1. > > - fsl,channel: The output channel that all input IRQs should be steered into. > > -- fsl,irq-groups: Number of IRQ groups managed by this controller instance. > > - Each group manages 64 input interrupts. > > +- fsl,irqs-per-chan: Number of input interrupts per channel. Should > > +be multiple of 32 > > + input interrupts and up to 512 interrupts. > > > > Example: > > > > @@ -28,7 +28,7 @@ Example: > > > clocks = <&clk IMX8MQ_CLK_DISP_APB_ROOT>; > > > clock-names = "ipg"; > > > fsl,channel = <0>; > > > - fsl,irq-groups = <1>; > > > + fsl,irqs-per-chan= <64>; > > > interrupt-controller; > > > #interrupt-cells = <1>; > > > };
> From: Marc Zyngier [mailto:marc.zyngier@arm.com] > Sent: Friday, January 18, 2019 5:39 PM > On 18/01/2019 08:48, Lucas Stach wrote: > > Am Freitag, den 18.01.2019, 07:53 +0000 schrieb Aisheng Dong: > >> Not all 64 interrupts may be used in one group. e.g. most irqsteer in > >> imx8qxp and imx8qm subsystems supports only 32 interrupts. > >> > >> As the IP integration parameters are Channel number and interrupts > >> number, let's use fsl,irqs-per-chan to represents how many interrupts > >> supported by this irqsteer channel. > > > > Sorry, but total NACK. I've got to great lengths with dumping the > > actually implemented register layout on i.MX8M and AFAICS the IRQs are > > always managed in groups of 64 IRQs, even if less than that are > > connected as input IRQs. This is what the actually present register > > set on i.MX8M tells us. > > Also, I'd really like the DT bindings not to change at every release. So whatever > change (if any) has to be done for this driver to support existing HW, please > make sure that the DT bindings are kept as stable as possible. > Sorry I should clarify it a bit. There's still no users in Devicetree. So I guess we can update it, right? Or not? Regards Dong Aisheng > Thanks, > > M. > -- > Jazz is not dead. It just smells funny...
On 18/01/2019 09:46, Aisheng Dong wrote: >> From: Marc Zyngier [mailto:marc.zyngier@arm.com] >> Sent: Friday, January 18, 2019 5:39 PM >> On 18/01/2019 08:48, Lucas Stach wrote: >>> Am Freitag, den 18.01.2019, 07:53 +0000 schrieb Aisheng Dong: >>>> Not all 64 interrupts may be used in one group. e.g. most irqsteer in >>>> imx8qxp and imx8qm subsystems supports only 32 interrupts. >>>> >>>> As the IP integration parameters are Channel number and interrupts >>>> number, let's use fsl,irqs-per-chan to represents how many interrupts >>>> supported by this irqsteer channel. >>> >>> Sorry, but total NACK. I've got to great lengths with dumping the >>> actually implemented register layout on i.MX8M and AFAICS the IRQs are >>> always managed in groups of 64 IRQs, even if less than that are >>> connected as input IRQs. This is what the actually present register >>> set on i.MX8M tells us. >> >> Also, I'd really like the DT bindings not to change at every release. So whatever >> change (if any) has to be done for this driver to support existing HW, please >> make sure that the DT bindings are kept as stable as possible. >> > > Sorry I should clarify it a bit. > There's still no users in Devicetree. > So I guess we can update it, right? Or not? What do you mean by no users? This driver is in 5.0, and I assume people are using it one way or another. Not having a platform in the kernel tree is pretty much irrelevant, as the kernel tree is not a canonical repository of existing platforms. Thanks, M.
> From: Marc Zyngier [mailto:marc.zyngier@arm.com] > Sent: Friday, January 18, 2019 6:12 PM [...] > >> From: Marc Zyngier [mailto:marc.zyngier@arm.com] > >> Sent: Friday, January 18, 2019 5:39 PM On 18/01/2019 08:48, Lucas > >> Stach wrote: > >>> Am Freitag, den 18.01.2019, 07:53 +0000 schrieb Aisheng Dong: > >>>> Not all 64 interrupts may be used in one group. e.g. most irqsteer > >>>> in imx8qxp and imx8qm subsystems supports only 32 interrupts. > >>>> > >>>> As the IP integration parameters are Channel number and interrupts > >>>> number, let's use fsl,irqs-per-chan to represents how many > >>>> interrupts supported by this irqsteer channel. > >>> > >>> Sorry, but total NACK. I've got to great lengths with dumping the > >>> actually implemented register layout on i.MX8M and AFAICS the IRQs > >>> are always managed in groups of 64 IRQs, even if less than that are > >>> connected as input IRQs. This is what the actually present register > >>> set on i.MX8M tells us. > >> > >> Also, I'd really like the DT bindings not to change at every release. > >> So whatever change (if any) has to be done for this driver to support > >> existing HW, please make sure that the DT bindings are kept as stable as > possible. > >> > > > > Sorry I should clarify it a bit. > > There's still no users in Devicetree. > > So I guess we can update it, right? Or not? > > What do you mean by no users? This driver is in 5.0, and I assume people are > using it one way or another. Not having a platform in the kernel tree is pretty > much irrelevant, as the kernel tree is not a canonical repository of existing > platforms. > I understand the concern. Theoretically yes, but it's very unlikely that there's already an out of tree users wants to use it for a long term as we're still at the very initial stage. And the most important reason is that current using actually is wrong. We can also choose to mark it as 'depreciated' and keep the backward compatibility in driver, but I'm not sure whether it's worthy to do it as we may add a lot ugly code in driver benefits no users. Ideas? Regards Dong Aisheng > Thanks, > > M. > -- > Jazz is not dead. It just smells funny...
Am Dienstag, den 22.01.2019, 10:56 +0000 schrieb Aisheng Dong: > > > > From: Marc Zyngier [mailto:marc.zyngier@arm.com] > > Sent: Friday, January 18, 2019 6:12 PM > > [...] > > > > > > > > From: Marc Zyngier [mailto:marc.zyngier@arm.com] > > > > Sent: Friday, January 18, 2019 5:39 PM On 18/01/2019 08:48, Lucas > > > > Stach wrote: > > > > > Am Freitag, den 18.01.2019, 07:53 +0000 schrieb Aisheng Dong: > > > > > > Not all 64 interrupts may be used in one group. e.g. most irqsteer > > > > > > in imx8qxp and imx8qm subsystems supports only 32 interrupts. > > > > > > > > > > > > As the IP integration parameters are Channel number and interrupts > > > > > > number, let's use fsl,irqs-per-chan to represents how many > > > > > > interrupts supported by this irqsteer channel. > > > > > > > > > > Sorry, but total NACK. I've got to great lengths with dumping the > > > > > actually implemented register layout on i.MX8M and AFAICS the IRQs > > > > > are always managed in groups of 64 IRQs, even if less than that are > > > > > connected as input IRQs. This is what the actually present register > > > > > set on i.MX8M tells us. > > > > > > > > Also, I'd really like the DT bindings not to change at every release. > > > > So whatever change (if any) has to be done for this driver to support > > > > existing HW, please make sure that the DT bindings are kept as stable as > > > > possible. > > > > > > > > > > Sorry I should clarify it a bit. > > > There's still no users in Devicetree. > > > So I guess we can update it, right? Or not? > > > > What do you mean by no users? This driver is in 5.0, and I assume people are > > using it one way or another. Not having a platform in the kernel tree is pretty > > much irrelevant, as the kernel tree is not a canonical repository of existing > > platforms. > > > > I understand the concern. > Theoretically yes, but it's very unlikely that there's already an out of tree users > wants to use it for a long term as we're still at the very initial stage. > > And the most important reason is that current using actually is wrong. > We can also choose to mark it as 'depreciated' and keep the backward compatibility in driver, > but I'm not sure whether it's worthy to do it as we may add a lot ugly code in driver > benefits no users. > > Ideas? I'm all for doing a breaking DT change now. The binding is significantly different from the downstream one anyways and I'm not aware of any upstream users that wouldn't be able to cope with a change at this point. I want to reach a conclusion on the discussion about how the HW actually works and is configured in reply to Patch 4/4 first. Regards, Lucas
On Tue, 22 Jan 2019 10:56:42 +0000, Aisheng Dong <aisheng.dong@nxp.com> wrote: > > > From: Marc Zyngier [mailto:marc.zyngier@arm.com] > > Sent: Friday, January 18, 2019 6:12 PM > [...] > > >> From: Marc Zyngier [mailto:marc.zyngier@arm.com] > > >> Sent: Friday, January 18, 2019 5:39 PM On 18/01/2019 08:48, Lucas > > >> Stach wrote: > > >>> Am Freitag, den 18.01.2019, 07:53 +0000 schrieb Aisheng Dong: > > >>>> Not all 64 interrupts may be used in one group. e.g. most irqsteer > > >>>> in imx8qxp and imx8qm subsystems supports only 32 interrupts. > > >>>> > > >>>> As the IP integration parameters are Channel number and interrupts > > >>>> number, let's use fsl,irqs-per-chan to represents how many > > >>>> interrupts supported by this irqsteer channel. > > >>> > > >>> Sorry, but total NACK. I've got to great lengths with dumping the > > >>> actually implemented register layout on i.MX8M and AFAICS the IRQs > > >>> are always managed in groups of 64 IRQs, even if less than that are > > >>> connected as input IRQs. This is what the actually present register > > >>> set on i.MX8M tells us. > > >> > > >> Also, I'd really like the DT bindings not to change at every release. > > >> So whatever change (if any) has to be done for this driver to support > > >> existing HW, please make sure that the DT bindings are kept as stable as > > possible. > > >> > > > > > > Sorry I should clarify it a bit. > > > There's still no users in Devicetree. > > > So I guess we can update it, right? Or not? > > > > What do you mean by no users? This driver is in 5.0, and I assume people are > > using it one way or another. Not having a platform in the kernel tree is pretty > > much irrelevant, as the kernel tree is not a canonical repository of existing > > platforms. > > > > I understand the concern. > Theoretically yes, but it's very unlikely that there's already an out of tree users > wants to use it for a long term as we're still at the very initial stage. > > And the most important reason is that current using actually is wrong. > We can also choose to mark it as 'depreciated' and keep the backward compatibility in driver, > but I'm not sure whether it's worthy to do it as we may add a lot ugly code in driver > benefits no users. > > Ideas? At this stage, and given that there is no consensus on how this driver should work, I'm tempted to just rip it out entirely by reverting 0136afa08967 and be done with it until people work out a way forward. M.
diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt b/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt index 45790ce..eaabcda 100644 --- a/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt @@ -16,8 +16,8 @@ Required properties: - #interrupt-cells: Specifies the number of cells needed to encode an interrupt source. The value must be 1. - fsl,channel: The output channel that all input IRQs should be steered into. -- fsl,irq-groups: Number of IRQ groups managed by this controller instance. - Each group manages 64 input interrupts. +- fsl,irqs-per-chan: Number of input interrupts per channel. Should be multiple of 32 + input interrupts and up to 512 interrupts. Example: @@ -28,7 +28,7 @@ Example: clocks = <&clk IMX8MQ_CLK_DISP_APB_ROOT>; clock-names = "ipg"; fsl,channel = <0>; - fsl,irq-groups = <1>; + fsl,irqs-per-chan= <64>; interrupt-controller; #interrupt-cells = <1>; };
Not all 64 interrupts may be used in one group. e.g. most irqsteer in imx8qxp and imx8qm subsystems supports only 32 interrupts. As the IP integration parameters are Channel number and interrupts number, let's use fsl,irqs-per-chan to represents how many interrupts supported by this irqsteer channel. Cc: Marc Zyngier <marc.zyngier@arm.com> Cc: Rob Herring <robh+dt@kernel.org> Cc: Lucas Stach <l.stach@pengutronix.de> Cc: Shawn Guo <shawnguo@kernel.org> Cc: devicetree@vger.kernel.org Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> --- .../devicetree/bindings/interrupt-controller/fsl,irqsteer.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)