| Message ID | 1532684881-19310-6-git-send-email-Bharat.Bhushan@nxp.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Scott Wood |
| Headers | show |
| Series | powerpc/mpic: Add non-contiguous interrupt sources | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/checkpatch | success | Test checkpatch on branch next |
| snowpatch_ozlabs/build-ppc64le | success | Test build-ppc64le on branch next |
| snowpatch_ozlabs/build-ppc64be | success | Test build-ppc64be on branch next |
| snowpatch_ozlabs/build-ppc64e | success | Test build-ppc64e on branch next |
| snowpatch_ozlabs/build-ppc32 | success | Test build-ppc32 on branch next |
On Fri, 2018-07-27 at 15:18 +0530, Bharat Bhushan wrote: > MPIC on NXP (Freescale) P2020 supports following irq > ranges: > > 0 - 11 (External interrupt) > > 16 - 79 (Internal interrupt) > > 176 - 183 (Messaging interrupt) > > 224 - 231 (Shared message signaled interrupt) Why don't you convert to the 4-cell interrupt specifiers that make dealing with these ranges less error-prone? > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > index 1006950..49ff348 100644 > --- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > +++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > @@ -57,6 +57,11 @@ void __init mpc85xx_rdb_pic_init(void) > MPIC_BIG_ENDIAN | > MPIC_SINGLE_DEST_CPU, > 0, 256, " OpenPIC "); > + } else if (of_machine_is_compatible("fsl,P2020RDB-PC")) { > + mpic = mpic_alloc(NULL, 0, > + MPIC_BIG_ENDIAN | > + MPIC_SINGLE_DEST_CPU, > + 0, 0, " OpenPIC "); > } else { > mpic = mpic_alloc(NULL, 0, > MPIC_BIG_ENDIAN | I don't think we want to grow a list of every single revision of every board in these platform files. -Scott
> -----Original Message----- > From: Scott Wood [mailto:oss@buserror.net] > Sent: Wednesday, August 8, 2018 2:44 AM > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au; > galak@kernel.crashing.org; mark.rutland@arm.com; > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org; > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > kernel@vger.kernel.org > Cc: robh@kernel.org; keescook@chromium.org; tyreld@linux.vnet.ibm.com; > joe@perches.com > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for P2020 > > On Fri, 2018-07-27 at 15:18 +0530, Bharat Bhushan wrote: > > MPIC on NXP (Freescale) P2020 supports following irq > > ranges: > > > 0 - 11 (External interrupt) > > > 16 - 79 (Internal interrupt) > > > 176 - 183 (Messaging interrupt) > > > 224 - 231 (Shared message signaled interrupt) > > Why don't you convert to the 4-cell interrupt specifiers that make dealing > with these ranges less error-prone? Ok , will do if we agree to have this series as per comment on other patch. > > > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > > b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > > index 1006950..49ff348 100644 > > --- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > > +++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > > @@ -57,6 +57,11 @@ void __init mpc85xx_rdb_pic_init(void) > > MPIC_BIG_ENDIAN | > > MPIC_SINGLE_DEST_CPU, > > 0, 256, " OpenPIC "); > > + } else if (of_machine_is_compatible("fsl,P2020RDB-PC")) { > > + mpic = mpic_alloc(NULL, 0, > > + MPIC_BIG_ENDIAN | > > + MPIC_SINGLE_DEST_CPU, > > + 0, 0, " OpenPIC "); > > } else { > > mpic = mpic_alloc(NULL, 0, > > MPIC_BIG_ENDIAN | > > I don't think we want to grow a list of every single revision of every board in > these platform files. One other confusing observation I have is that "irq_count" from platform code is given precedence over "last-interrupt-source" in device-tree. Should not device-tree should have precedence otherwise there is no point using " last-interrupt-source" if platform code passes "irq_count" in mpic_alloc(). Thanks -Bharat > > -Scott
On Wed, 2018-08-08 at 03:44 +0000, Bharat Bhushan wrote: > > -----Original Message----- > > From: Scott Wood [mailto:oss@buserror.net] > > Sent: Wednesday, August 8, 2018 2:44 AM > > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au; > > galak@kernel.crashing.org; mark.rutland@arm.com; > > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org; > > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > > kernel@vger.kernel.org > > Cc: robh@kernel.org; keescook@chromium.org; tyreld@linux.vnet.ibm.com; > > joe@perches.com > > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for P2020 > > > > On Fri, 2018-07-27 at 15:18 +0530, Bharat Bhushan wrote: > > > MPIC on NXP (Freescale) P2020 supports following irq > > > ranges: > > > > 0 - 11 (External interrupt) > > > > 16 - 79 (Internal interrupt) > > > > 176 - 183 (Messaging interrupt) > > > > 224 - 231 (Shared message signaled interrupt) > > > > Why don't you convert to the 4-cell interrupt specifiers that make dealing > > with these ranges less error-prone? > > Ok , will do if we agree to have this series as per comment on other patch. If you're concerned with errors, this would be a good things to do regardless. Actually, it seems that p2020si-post.dtsi already uses 4-cell interrupts. What is motivating this patchset? Is there something wrong in the existing dts files? > > > > > > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > > > b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > > > index 1006950..49ff348 100644 > > > --- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > > > +++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > > > @@ -57,6 +57,11 @@ void __init mpc85xx_rdb_pic_init(void) > > > MPIC_BIG_ENDIAN | > > > MPIC_SINGLE_DEST_CPU, > > > 0, 256, " OpenPIC "); > > > + } else if (of_machine_is_compatible("fsl,P2020RDB-PC")) { > > > + mpic = mpic_alloc(NULL, 0, > > > + MPIC_BIG_ENDIAN | > > > + MPIC_SINGLE_DEST_CPU, > > > + 0, 0, " OpenPIC "); > > > } else { > > > mpic = mpic_alloc(NULL, 0, > > > MPIC_BIG_ENDIAN | > > > > I don't think we want to grow a list of every single revision of every > > board in > > these platform files. > > One other confusing observation I have is that "irq_count" from platform > code is given precedence over "last-interrupt-source" in device-tree. > Should not device-tree should have precedence otherwise there is no point > using " last-interrupt-source" if platform code passes "irq_count" in > mpic_alloc(). Maybe, though I don't think it matters much given that last-interrupt-source was only added to avoid having to pass irq_count in platform code. -Scott
> -----Original Message----- > From: Scott Wood [mailto:oss@buserror.net] > Sent: Wednesday, August 8, 2018 11:26 AM > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au; > galak@kernel.crashing.org; mark.rutland@arm.com; > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org; > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > kernel@vger.kernel.org > Cc: robh@kernel.org; keescook@chromium.org; tyreld@linux.vnet.ibm.com; > joe@perches.com > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for P2020 > > On Wed, 2018-08-08 at 03:44 +0000, Bharat Bhushan wrote: > > > -----Original Message----- > > > From: Scott Wood [mailto:oss@buserror.net] > > > Sent: Wednesday, August 8, 2018 2:44 AM > > > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > > > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au; > > > galak@kernel.crashing.org; mark.rutland@arm.com; > > > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org; > > > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > > > kernel@vger.kernel.org > > > Cc: robh@kernel.org; keescook@chromium.org; > > > tyreld@linux.vnet.ibm.com; joe@perches.com > > > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for > > > P2020 > > > > > > On Fri, 2018-07-27 at 15:18 +0530, Bharat Bhushan wrote: > > > > MPIC on NXP (Freescale) P2020 supports following irq > > > > ranges: > > > > > 0 - 11 (External interrupt) > > > > > 16 - 79 (Internal interrupt) > > > > > 176 - 183 (Messaging interrupt) > > > > > 224 - 231 (Shared message signaled interrupt) > > > > > > Why don't you convert to the 4-cell interrupt specifiers that make > > > dealing with these ranges less error-prone? > > > > Ok , will do if we agree to have this series as per comment on other patch. > > If you're concerned with errors, this would be a good things to do regardless. > Actually, it seems that p2020si-post.dtsi already uses 4-cell interrupts. > > What is motivating this patchset? Is there something wrong in the existing > dts files? There is no error in device tree. Main motivation is to improve code for following reasons: - While code study it was found that if a reserved irq-number used then there are no check in driver. irq will be configured as correct and interrupt will never fire. - Warnings were observed on development platform (simulator) when read/write to reserved MPIC reason during init. > > > > > > > > > > > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > > > > b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > > > > index 1006950..49ff348 100644 > > > > --- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > > > > +++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c > > > > @@ -57,6 +57,11 @@ void __init mpc85xx_rdb_pic_init(void) > > > > MPIC_BIG_ENDIAN | > > > > MPIC_SINGLE_DEST_CPU, > > > > 0, 256, " OpenPIC "); > > > > + } else if (of_machine_is_compatible("fsl,P2020RDB-PC")) { > > > > + mpic = mpic_alloc(NULL, 0, > > > > + MPIC_BIG_ENDIAN | > > > > + MPIC_SINGLE_DEST_CPU, > > > > + 0, 0, " OpenPIC "); > > > > } else { > > > > mpic = mpic_alloc(NULL, 0, > > > > MPIC_BIG_ENDIAN | > > > > > > I don't think we want to grow a list of every single revision of > > > every board in these platform files. > > > > One other confusing observation I have is that "irq_count" from > > platform code is given precedence over "last-interrupt-source" in device- > tree. > > Should not device-tree should have precedence otherwise there is no > > point using " last-interrupt-source" if platform code passes > > "irq_count" in mpic_alloc(). > > Maybe, though I don't think it matters much given that last-interrupt-source > was only added to avoid having to pass irq_count in platform code. Thanks for clarifying; My understanding was that "last-interrupt-source" added to ensure that we can over-ride value passed from platform code. In that case we do not need to change code and can control from device tree. Thanks -Bharat > > -Scott
On Wed, 2018-08-08 at 06:28 +0000, Bharat Bhushan wrote: > > -----Original Message----- > > From: Scott Wood [mailto:oss@buserror.net] > > Sent: Wednesday, August 8, 2018 11:26 AM > > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au; > > galak@kernel.crashing.org; mark.rutland@arm.com; > > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org; > > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > > kernel@vger.kernel.org > > Cc: robh@kernel.org; keescook@chromium.org; tyreld@linux.vnet.ibm.com; > > joe@perches.com > > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for P2020 > > > > On Wed, 2018-08-08 at 03:44 +0000, Bharat Bhushan wrote: > > > > -----Original Message----- > > > > From: Scott Wood [mailto:oss@buserror.net] > > > > Sent: Wednesday, August 8, 2018 2:44 AM > > > > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > > > > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au; > > > > galak@kernel.crashing.org; mark.rutland@arm.com; > > > > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org; > > > > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > > > > kernel@vger.kernel.org > > > > Cc: robh@kernel.org; keescook@chromium.org; > > > > tyreld@linux.vnet.ibm.com; joe@perches.com > > > > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for > > > > P2020 > > > > > > > > On Fri, 2018-07-27 at 15:18 +0530, Bharat Bhushan wrote: > > > > > MPIC on NXP (Freescale) P2020 supports following irq > > > > > ranges: > > > > > > 0 - 11 (External interrupt) > > > > > > 16 - 79 (Internal interrupt) > > > > > > 176 - 183 (Messaging interrupt) > > > > > > 224 - 231 (Shared message signaled interrupt) > > > > > > > > Why don't you convert to the 4-cell interrupt specifiers that make > > > > dealing with these ranges less error-prone? > > > > > > Ok , will do if we agree to have this series as per comment on other > > > patch. > > > > If you're concerned with errors, this would be a good things to do > > regardless. > > Actually, it seems that p2020si-post.dtsi already uses 4-cell interrupts. > > > > What is motivating this patchset? Is there something wrong in the > > existing > > dts files? > > There is no error in device tree. Main motivation is to improve code for > following reasons: > - While code study it was found that if a reserved irq-number used then > there are no check in driver. irq will be configured as correct and > interrupt will never fire. Again, a wrong interrupt number won't fire, whether an interrupt by that number exists or not. I wouldn't mind a sanity check in the driver if the programming model made it properly discoverable, but I don't think it's worth messing with device trees just for this (and even less so given that there don't seem to be new chips coming out that this would be relevant for). > > > One other confusing observation I have is that "irq_count" from > > > platform code is given precedence over "last-interrupt-source" in > > > device- > > > > tree. > > > Should not device-tree should have precedence otherwise there is no > > > point using " last-interrupt-source" if platform code passes > > > "irq_count" in mpic_alloc(). > > > > Maybe, though I don't think it matters much given that last-interrupt- > > source > > was only added to avoid having to pass irq_count in platform code. > > Thanks for clarifying; > > My understanding was that "last-interrupt-source" added to ensure that we > can over-ride value passed from platform code. In that case we do not need > to change code and can control from device tree. The changelog says, "To avoid needing to write custom board-specific code to detect that scenario, allow it to be easily overridden in the device-tree," where "it" means the value provided by hardware. The goal was to pass in 256 without board code in the kernel, not to override the 256. -Scott
> -----Original Message----- > From: Scott Wood [mailto:oss@buserror.net] > Sent: Wednesday, August 8, 2018 11:27 PM > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au; > galak@kernel.crashing.org; mark.rutland@arm.com; > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org; > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > kernel@vger.kernel.org > Cc: robh@kernel.org; keescook@chromium.org; tyreld@linux.vnet.ibm.com; > joe@perches.com > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for P2020 > > On Wed, 2018-08-08 at 06:28 +0000, Bharat Bhushan wrote: > > > -----Original Message----- > > > From: Scott Wood [mailto:oss@buserror.net] > > > Sent: Wednesday, August 8, 2018 11:26 AM > > > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > > > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au; > > > galak@kernel.crashing.org; mark.rutland@arm.com; > > > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org; > > > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > > > kernel@vger.kernel.org > > > Cc: robh@kernel.org; keescook@chromium.org; > > > tyreld@linux.vnet.ibm.com; joe@perches.com > > > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for > > > P2020 > > > > > > On Wed, 2018-08-08 at 03:44 +0000, Bharat Bhushan wrote: > > > > > -----Original Message----- > > > > > From: Scott Wood [mailto:oss@buserror.net] > > > > > Sent: Wednesday, August 8, 2018 2:44 AM > > > > > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > > > > > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au; > > > > > galak@kernel.crashing.org; mark.rutland@arm.com; > > > > > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org; > > > > > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; > > > > > linux- kernel@vger.kernel.org > > > > > Cc: robh@kernel.org; keescook@chromium.org; > > > > > tyreld@linux.vnet.ibm.com; joe@perches.com > > > > > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for > > > > > P2020 > > > > > > > > > > On Fri, 2018-07-27 at 15:18 +0530, Bharat Bhushan wrote: > > > > > > MPIC on NXP (Freescale) P2020 supports following irq > > > > > > ranges: > > > > > > > 0 - 11 (External interrupt) > > > > > > > 16 - 79 (Internal interrupt) > > > > > > > 176 - 183 (Messaging interrupt) > > > > > > > 224 - 231 (Shared message signaled interrupt) > > > > > > > > > > Why don't you convert to the 4-cell interrupt specifiers that > > > > > make dealing with these ranges less error-prone? > > > > > > > > Ok , will do if we agree to have this series as per comment on > > > > other patch. > > > > > > If you're concerned with errors, this would be a good things to do > > > regardless. > > > Actually, it seems that p2020si-post.dtsi already uses 4-cell interrupts. > > > > > > What is motivating this patchset? Is there something wrong in the > > > existing dts files? > > > > There is no error in device tree. Main motivation is to improve code > > for following reasons: > > - While code study it was found that if a reserved irq-number used > > then there are no check in driver. irq will be configured as correct > > and interrupt will never fire. > > Again, a wrong interrupt number won't fire, whether an interrupt by that > number exists or not. I wouldn't mind a sanity check in the driver if the > programming model made it properly discoverable, but I don't think it's > worth messing with device trees just for this (and even less so given that > there don't seem to be new chips coming out that this would be relevant > for). Fair enough, we can use MPIC version to define supported interrupts ranges. Will that be acceptable. Thanks -Bharat > > > > > One other confusing observation I have is that "irq_count" from > > > > platform code is given precedence over "last-interrupt-source" in > > > > device- > > > > > > tree. > > > > Should not device-tree should have precedence otherwise there is > > > > no point using " last-interrupt-source" if platform code passes > > > > "irq_count" in mpic_alloc(). > > > > > > Maybe, though I don't think it matters much given that > > > last-interrupt- source was only added to avoid having to pass > > > irq_count in platform code. > > > > Thanks for clarifying; > > > > My understanding was that "last-interrupt-source" added to ensure that > > we can over-ride value passed from platform code. In that case we do > > not need to change code and can control from device tree. > > The changelog says, "To avoid needing to write custom board-specific code > to detect that scenario, allow it to be easily overridden in the device-tree," > where "it" means the value provided by hardware. The goal was to pass in > 256 without board code in the kernel, not to override the 256. > > -Scott
On Thu, 2018-08-09 at 03:28 +0000, Bharat Bhushan wrote: > > -----Original Message----- > > From: Scott Wood [mailto:oss@buserror.net] > > Sent: Wednesday, August 8, 2018 11:27 PM > > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au; > > galak@kernel.crashing.org; mark.rutland@arm.com; > > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org; > > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > > kernel@vger.kernel.org > > Cc: robh@kernel.org; keescook@chromium.org; tyreld@linux.vnet.ibm.com; > > joe@perches.com > > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for P2020 > > > > On Wed, 2018-08-08 at 06:28 +0000, Bharat Bhushan wrote: > > > > -----Original Message----- > > > > From: Scott Wood [mailto:oss@buserror.net] > > > > Sent: Wednesday, August 8, 2018 11:26 AM > > > > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > > > > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au; > > > > galak@kernel.crashing.org; mark.rutland@arm.com; > > > > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org; > > > > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > > > > kernel@vger.kernel.org > > > > Cc: robh@kernel.org; keescook@chromium.org; > > > > tyreld@linux.vnet.ibm.com; joe@perches.com > > > > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for > > > > P2020 > > > > > > > > On Wed, 2018-08-08 at 03:44 +0000, Bharat Bhushan wrote: > > > > > > -----Original Message----- > > > > > > From: Scott Wood [mailto:oss@buserror.net] > > > > > > Sent: Wednesday, August 8, 2018 2:44 AM > > > > > > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > > > > > > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au; > > > > > > galak@kernel.crashing.org; mark.rutland@arm.com; > > > > > > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org; > > > > > > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; > > > > > > linux- kernel@vger.kernel.org > > > > > > Cc: robh@kernel.org; keescook@chromium.org; > > > > > > tyreld@linux.vnet.ibm.com; joe@perches.com > > > > > > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for > > > > > > P2020 > > > > > > > > > > > > On Fri, 2018-07-27 at 15:18 +0530, Bharat Bhushan wrote: > > > > > > > MPIC on NXP (Freescale) P2020 supports following irq > > > > > > > ranges: > > > > > > > > 0 - 11 (External interrupt) > > > > > > > > 16 - 79 (Internal interrupt) > > > > > > > > 176 - 183 (Messaging interrupt) > > > > > > > > 224 - 231 (Shared message signaled interrupt) > > > > > > > > > > > > Why don't you convert to the 4-cell interrupt specifiers that > > > > > > make dealing with these ranges less error-prone? > > > > > > > > > > Ok , will do if we agree to have this series as per comment on > > > > > other patch. > > > > > > > > If you're concerned with errors, this would be a good things to do > > > > regardless. > > > > Actually, it seems that p2020si-post.dtsi already uses 4-cell > > > > interrupts. > > > > > > > > What is motivating this patchset? Is there something wrong in the > > > > existing dts files? > > > > > > There is no error in device tree. Main motivation is to improve code > > > for following reasons: > > > - While code study it was found that if a reserved irq-number used > > > then there are no check in driver. irq will be configured as correct > > > and interrupt will never fire. > > > > Again, a wrong interrupt number won't fire, whether an interrupt by that > > number exists or not. I wouldn't mind a sanity check in the driver if the > > programming model made it properly discoverable, but I don't think it's > > worth messing with device trees just for this (and even less so given that > > there don't seem to be new chips coming out that this would be relevant > > for). > > Fair enough, we can use MPIC version to define supported interrupts ranges. > Will that be acceptable. It's better than device tree changes but I'm not convinced it's worthwhile just to suppress some simulator warnings. If the warnings really bother you, you can use pic-no-reset in the device tree (assuming this isn't some new chip that you want to make sure doesn't fall over when the usual mpic init happens) and/or convince the hardware people to make the interface properly discoverable including discontiguous regions (if there *is* some new chip I haven't heard about). -Scott
> -----Original Message----- > From: Scott Wood [mailto:oss@buserror.net] > Sent: Thursday, August 9, 2018 11:42 AM > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au; > galak@kernel.crashing.org; mark.rutland@arm.com; > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org; > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > kernel@vger.kernel.org > Cc: robh@kernel.org; keescook@chromium.org; tyreld@linux.vnet.ibm.com; > joe@perches.com > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for P2020 > > On Thu, 2018-08-09 at 03:28 +0000, Bharat Bhushan wrote: > > > -----Original Message----- > > > From: Scott Wood [mailto:oss@buserror.net] > > > Sent: Wednesday, August 8, 2018 11:27 PM > > > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > > > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au; > > > galak@kernel.crashing.org; mark.rutland@arm.com; > > > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org; > > > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > > > kernel@vger.kernel.org > > > Cc: robh@kernel.org; keescook@chromium.org; > > > tyreld@linux.vnet.ibm.com; joe@perches.com > > > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for > > > P2020 > > > > > > On Wed, 2018-08-08 at 06:28 +0000, Bharat Bhushan wrote: > > > > > -----Original Message----- > > > > > From: Scott Wood [mailto:oss@buserror.net] > > > > > Sent: Wednesday, August 8, 2018 11:26 AM > > > > > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > > > > > benh@kernel.crashing.org; paulus@samba.org; mpe@ellerman.id.au; > > > > > galak@kernel.crashing.org; mark.rutland@arm.com; > > > > > kstewart@linuxfoundation.org; gregkh@linuxfoundation.org; > > > > > devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; > > > > > linux- kernel@vger.kernel.org > > > > > Cc: robh@kernel.org; keescook@chromium.org; > > > > > tyreld@linux.vnet.ibm.com; joe@perches.com > > > > > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges for > > > > > P2020 > > > > > > > > > > On Wed, 2018-08-08 at 03:44 +0000, Bharat Bhushan wrote: > > > > > > > -----Original Message----- > > > > > > > From: Scott Wood [mailto:oss@buserror.net] > > > > > > > Sent: Wednesday, August 8, 2018 2:44 AM > > > > > > > To: Bharat Bhushan <bharat.bhushan@nxp.com>; > > > > > > > benh@kernel.crashing.org; paulus@samba.org; > > > > > > > mpe@ellerman.id.au; galak@kernel.crashing.org; > > > > > > > mark.rutland@arm.com; kstewart@linuxfoundation.org; > > > > > > > gregkh@linuxfoundation.org; devicetree@vger.kernel.org; > > > > > > > linuxppc-dev@lists.ozlabs.org; > > > > > > > linux- kernel@vger.kernel.org > > > > > > > Cc: robh@kernel.org; keescook@chromium.org; > > > > > > > tyreld@linux.vnet.ibm.com; joe@perches.com > > > > > > > Subject: Re: [RFC 5/5] powerpc/fsl: Add supported-irq-ranges > > > > > > > for > > > > > > > P2020 > > > > > > > > > > > > > > On Fri, 2018-07-27 at 15:18 +0530, Bharat Bhushan wrote: > > > > > > > > MPIC on NXP (Freescale) P2020 supports following irq > > > > > > > > ranges: > > > > > > > > > 0 - 11 (External interrupt) > > > > > > > > > 16 - 79 (Internal interrupt) > > > > > > > > > 176 - 183 (Messaging interrupt) > > > > > > > > > 224 - 231 (Shared message signaled interrupt) > > > > > > > > > > > > > > Why don't you convert to the 4-cell interrupt specifiers > > > > > > > that make dealing with these ranges less error-prone? > > > > > > > > > > > > Ok , will do if we agree to have this series as per comment on > > > > > > other patch. > > > > > > > > > > If you're concerned with errors, this would be a good things to > > > > > do regardless. > > > > > Actually, it seems that p2020si-post.dtsi already uses 4-cell > > > > > interrupts. > > > > > > > > > > What is motivating this patchset? Is there something wrong in > > > > > the existing dts files? > > > > > > > > There is no error in device tree. Main motivation is to improve > > > > code for following reasons: > > > > - While code study it was found that if a reserved irq-number > > > > used then there are no check in driver. irq will be configured as > > > > correct and interrupt will never fire. > > > > > > Again, a wrong interrupt number won't fire, whether an interrupt by > > > that number exists or not. I wouldn't mind a sanity check in the > > > driver if the programming model made it properly discoverable, but I > > > don't think it's worth messing with device trees just for this (and > > > even less so given that there don't seem to be new chips coming out > > > that this would be relevant for). > > > > Fair enough, we can use MPIC version to define supported interrupts > ranges. > > Will that be acceptable. > > It's better than device tree changes but I'm not convinced it's worthwhile just > to suppress some simulator warnings. > If the warnings really bother you, you > can use pic-no-reset in the device tree (assuming this isn't some new chip > that you want to make sure doesn't fall over when the usual mpic init > happens) and/or convince the hardware people to make the interface > properly discoverable including discontiguous regions (if there *is* some > new chip I haven't heard about). There is new chip under development based on e200, which uses same mpic as in older PowerPC versions. This patch was not just about suppressing the warning but warning was the observation that reserved region is getting accessed during init/uninit/save/restore. This patch was just an minor improvement to avoid these accesses. We will drop this series if this improvement is not convincing. Thanks -Bharat > > -Scott
diff --git a/arch/powerpc/boot/dts/fsl/p2020si-post.dtsi b/arch/powerpc/boot/dts/fsl/p2020si-post.dtsi index 884e01b..08e266b 100644 --- a/arch/powerpc/boot/dts/fsl/p2020si-post.dtsi +++ b/arch/powerpc/boot/dts/fsl/p2020si-post.dtsi @@ -192,6 +192,9 @@ /include/ "pq3-sec3.1-0.dtsi" /include/ "pq3-mpic.dtsi" /include/ "pq3-mpic-timer-B.dtsi" + pic@40000 { + supported-irq-ranges = <0 11 16 79 176 183 224 231>; + }; global-utilities@e0000 { compatible = "fsl,p2020-guts"; diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c index 1006950..49ff348 100644 --- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c +++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c @@ -57,6 +57,11 @@ void __init mpc85xx_rdb_pic_init(void) MPIC_BIG_ENDIAN | MPIC_SINGLE_DEST_CPU, 0, 256, " OpenPIC "); + } else if (of_machine_is_compatible("fsl,P2020RDB-PC")) { + mpic = mpic_alloc(NULL, 0, + MPIC_BIG_ENDIAN | + MPIC_SINGLE_DEST_CPU, + 0, 0, " OpenPIC "); } else { mpic = mpic_alloc(NULL, 0, MPIC_BIG_ENDIAN |