| Message ID | 20220712140009.20765-1-quic_ppareek@quicinc.com |
|---|---|
| State | Accepted |
| Commit | 1189a9cf144a745e4b98ff4f6cf5f79ab0b56cfb |
| Headers | show |
| Series | [v2] arm64: dts: qcom: sc8280xp: fix apps_smmu irq | expand |
On Tue, Jul 12, 2022 at 07:30:09PM +0530, Parikshit Pareek wrote: > Wrong values have been introduced for interrupts property. Fix those > ones, and correct the mapping of context banks to irq number. > And you ignored my comment about sorting the IRQs... Thanks, Mani > Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform") > Signed-off-by: Parikshit Pareek <quic_ppareek@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > index 7945cbb57bb4..1276a833251e 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -1580,7 +1580,6 @@ > <GIC_SPI 412 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 706 IRQ_TYPE_LEVEL_HIGH>, > - <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>, > @@ -1591,6 +1590,7 @@ > <GIC_SPI 693 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 694 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 695 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 696 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 410 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 411 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 420 IRQ_TYPE_LEVEL_HIGH>, > -- > 2.17.1 >
On Tue, Jul 12, 2022 at 08:10:44PM +0530, Manivannan Sadhasivam wrote: > On Tue, Jul 12, 2022 at 07:30:09PM +0530, Parikshit Pareek wrote: > > Wrong values have been introduced for interrupts property. Fix those > > ones, and correct the mapping of context banks to irq number. > > > > And you ignored my comment about sorting the IRQs... My bad, will take care. > > Thanks, > Mani > > > Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform") > > Signed-off-by: Parikshit Pareek <quic_ppareek@quicinc.com> > > --- > > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > index 7945cbb57bb4..1276a833251e 100644 > > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > @@ -1580,7 +1580,6 @@ > > <GIC_SPI 412 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 706 IRQ_TYPE_LEVEL_HIGH>, > > - <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>, > > @@ -1591,6 +1590,7 @@ > > <GIC_SPI 693 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 694 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 695 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 696 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 410 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 411 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 420 IRQ_TYPE_LEVEL_HIGH>, > > -- > > 2.17.1 > > Regards, Parikshit Pareek
On Tue, Jul 12, 2022 at 07:30:09PM +0530, Parikshit Pareek wrote: > Wrong values have been introduced for interrupts property. Fix those > ones, and correct the mapping of context banks to irq number. > > Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform") > Signed-off-by: Parikshit Pareek <quic_ppareek@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > index 7945cbb57bb4..1276a833251e 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -1580,7 +1580,6 @@ > <GIC_SPI 412 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 706 IRQ_TYPE_LEVEL_HIGH>, > - <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>, > @@ -1591,6 +1590,7 @@ > <GIC_SPI 693 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 694 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 695 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 696 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 410 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 411 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 420 IRQ_TYPE_LEVEL_HIGH>, Not sure how that happened. Looks correct now, thanks. Reviewed-by: Johan Hovold <johan+linaro@kernel.org> Johan
On Tue, 12 Jul 2022 at 19:24, Johan Hovold <johan@kernel.org> wrote: > > On Tue, Jul 12, 2022 at 08:10:44PM +0530, Manivannan Sadhasivam wrote: > > On Tue, Jul 12, 2022 at 07:30:09PM +0530, Parikshit Pareek wrote: > > > Wrong values have been introduced for interrupts property. Fix those > > > ones, and correct the mapping of context banks to irq number. > > > > > > > And you ignored my comment about sorting the IRQs... > > Isn't the order significant here? Either way, that would be a separate > change that shouldn't be merged with the fix. I'd tend to agree here. Let's get the fix in first and sort the IRQs in a separate commit. The order of them is strange indeed.
On Tue, Jul 12, 2022 at 07:41:14PM +0300, Dmitry Baryshkov wrote: > On Tue, 12 Jul 2022 at 19:24, Johan Hovold <johan@kernel.org> wrote: > > > > On Tue, Jul 12, 2022 at 08:10:44PM +0530, Manivannan Sadhasivam wrote: > > > On Tue, Jul 12, 2022 at 07:30:09PM +0530, Parikshit Pareek wrote: > > > > Wrong values have been introduced for interrupts property. Fix those > > > > ones, and correct the mapping of context banks to irq number. > > > > > > > > > > And you ignored my comment about sorting the IRQs... > > > > Isn't the order significant here? Either way, that would be a separate > > change that shouldn't be merged with the fix. > > I'd tend to agree here. Let's get the fix in first and sort the IRQs > in a separate commit. The order of them is strange indeed. > Yeah, I was expecting a separate patch for the cleanup. Thanks, Mani > -- > With best wishes > Dmitry
Hi Mani, On 7/12/2022 11:56 PM, Manivannan Sadhasivam wrote: > On Tue, Jul 12, 2022 at 07:41:14PM +0300, Dmitry Baryshkov wrote: >> On Tue, 12 Jul 2022 at 19:24, Johan Hovold <johan@kernel.org> wrote: >>> On Tue, Jul 12, 2022 at 08:10:44PM +0530, Manivannan Sadhasivam wrote: >>>> On Tue, Jul 12, 2022 at 07:30:09PM +0530, Parikshit Pareek wrote: >>>>> Wrong values have been introduced for interrupts property. Fix those >>>>> ones, and correct the mapping of context banks to irq number. >>>>> >>>> And you ignored my comment about sorting the IRQs... >>> Isn't the order significant here? Either way, that would be a separate >>> change that shouldn't be merged with the fix. >> I'd tend to agree here. Let's get the fix in first and sort the IRQs >> in a separate commit. The order of them is strange indeed. >> As per "arm,smmu.yaml" devicetree documentation, context interrupts are specified in order of their indexing by the SMMU and not the IRQ numbers, quoting relevant part below. "Interrupt list, with the first #global-interrupts entries corresponding to the global interrupts and any following entries corresponding to context interrupts, specified in order of their indexing by the SMMU." And the current order in DT without sorting by IRQ number matches with the SMMU IP interrupt document, For example, in the current DT order, you see 409 and then 418 instead of 410. Here 409 is app_tcu_cxt_irpt_vec[73], 418 is app_tcu_cxt_irpt_vec[74] and 410 is app_tcu_cxt_irpt_vec[90] and hence the ordering of 409, 418 .... 410. Also the reverse ordering at the end from 913 to 891 is also as per this indexing. So the current ordering is proper and do not require sorting. As for the missing IRQs and duplicate ones, I will reply on the patch, looks like there are some other misconfigurations as well. Thanks, Sai
Hi Parikshit, On 7/12/2022 7:30 PM, Parikshit Pareek wrote: > Wrong values have been introduced for interrupts property. Fix those > ones, and correct the mapping of context banks to irq number. > > Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform") > Signed-off-by: Parikshit Pareek <quic_ppareek@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > index 7945cbb57bb4..1276a833251e 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -1580,7 +1580,6 @@ > <GIC_SPI 412 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 706 IRQ_TYPE_LEVEL_HIGH>, > - <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>, > @@ -1591,6 +1590,7 @@ > <GIC_SPI 693 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 694 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 695 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 696 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 410 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 411 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 420 IRQ_TYPE_LEVEL_HIGH>, Cross checking with the IP doc, 696 is not the only one missing, couple of other interrupts are also missing, below is the list. Interrupt 689 and 706 are not present in the IP doc, remove it. Interrupts 696, 697, 716, 913 are missing, I see this patch adds 696, add 697 as well. Interrupt 890 is not a context interrupt, remove it. Thanks, Sai
On 7/14/2022 12:47 PM, Sai Prakash Ranjan wrote: > Hi Parikshit, > > On 7/12/2022 7:30 PM, Parikshit Pareek wrote: >> Wrong values have been introduced for interrupts property. Fix those >> ones, and correct the mapping of context banks to irq number. >> >> Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform") >> Signed-off-by: Parikshit Pareek <quic_ppareek@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >> index 7945cbb57bb4..1276a833251e 100644 >> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >> @@ -1580,7 +1580,6 @@ >> <GIC_SPI 412 IRQ_TYPE_LEVEL_HIGH>, >> <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH>, >> <GIC_SPI 706 IRQ_TYPE_LEVEL_HIGH>, >> - <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>, >> <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>, >> <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>, >> <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>, >> @@ -1591,6 +1590,7 @@ >> <GIC_SPI 693 IRQ_TYPE_LEVEL_HIGH>, >> <GIC_SPI 694 IRQ_TYPE_LEVEL_HIGH>, >> <GIC_SPI 695 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 696 IRQ_TYPE_LEVEL_HIGH>, >> <GIC_SPI 410 IRQ_TYPE_LEVEL_HIGH>, >> <GIC_SPI 411 IRQ_TYPE_LEVEL_HIGH>, >> <GIC_SPI 420 IRQ_TYPE_LEVEL_HIGH>, > > Cross checking with the IP doc, 696 is not the only one missing, couple of other interrupts are also missing, below is the list. > > Interrupt 689 and 706 are not present in the IP doc, remove it. > Interrupts 696, 697, 716, 913 are missing, I see this patch adds 696, add 697 as well. > Interrupt 890 is not a context interrupt, remove it. > > Checking offline with Parikshit, looks like adding all these entries results in interrupt selftest failures. So for now, we can just have the original patch merged till these failures are debugged further with internal teams. Thanks, Sai
On 7/12/2022 7:30 PM, Parikshit Pareek wrote: > Wrong values have been introduced for interrupts property. Fix those > ones, and correct the mapping of context banks to irq number. > > Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform") > Signed-off-by: Parikshit Pareek <quic_ppareek@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > index 7945cbb57bb4..1276a833251e 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -1580,7 +1580,6 @@ > <GIC_SPI 412 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 706 IRQ_TYPE_LEVEL_HIGH>, > - <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>, > @@ -1591,6 +1590,7 @@ > <GIC_SPI 693 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 694 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 695 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 696 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 410 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 411 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 420 IRQ_TYPE_LEVEL_HIGH>, Reviewed-by: Sai Prakash Ranjan <quic_saipraka@quicinc.com> Thanks, Sai
On Thu, Jul 14, 2022 at 12:37:45PM +0530, Sai Prakash Ranjan wrote: > Hi Mani, > > On 7/12/2022 11:56 PM, Manivannan Sadhasivam wrote: > > On Tue, Jul 12, 2022 at 07:41:14PM +0300, Dmitry Baryshkov wrote: > > > On Tue, 12 Jul 2022 at 19:24, Johan Hovold <johan@kernel.org> wrote: > > > > On Tue, Jul 12, 2022 at 08:10:44PM +0530, Manivannan Sadhasivam wrote: > > > > > On Tue, Jul 12, 2022 at 07:30:09PM +0530, Parikshit Pareek wrote: > > > > > > Wrong values have been introduced for interrupts property. Fix those > > > > > > ones, and correct the mapping of context banks to irq number. > > > > > > > > > > > And you ignored my comment about sorting the IRQs... > > > > Isn't the order significant here? Either way, that would be a separate > > > > change that shouldn't be merged with the fix. > > > I'd tend to agree here. Let's get the fix in first and sort the IRQs > > > in a separate commit. The order of them is strange indeed. > > > > > As per "arm,smmu.yaml" devicetree documentation, context interrupts are specified in order of their indexing by the SMMU > and not the IRQ numbers, quoting relevant part below. > > "Interrupt list, with the first #global-interrupts entries corresponding to the global interrupts > and any following entries corresponding to context interrupts, specified in order of their indexing by the SMMU." > > And the current order in DT without sorting by IRQ number matches with the SMMU IP interrupt document, > > For example, in the current DT order, you see 409 and then 418 instead of 410. Here 409 is app_tcu_cxt_irpt_vec[73], > 418 is app_tcu_cxt_irpt_vec[74] and 410 is app_tcu_cxt_irpt_vec[90] and hence the ordering of 409, 418 .... 410. > Also the reverse ordering at the end from 913 to 891 is also as per this indexing. > > So the current ordering is proper and do not require sorting. > Ah, I missed reading the binding. Sorry for the noise. > As for the missing IRQs and duplicate ones, I will reply on the patch, looks like there are some other misconfigurations as well. > Thanks, Mani > Thanks, > Sai >
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi index 7945cbb57bb4..1276a833251e 100644 --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi @@ -1580,7 +1580,6 @@ <GIC_SPI 412 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 706 IRQ_TYPE_LEVEL_HIGH>, - <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>, @@ -1591,6 +1590,7 @@ <GIC_SPI 693 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 694 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 695 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 696 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 410 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 411 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 420 IRQ_TYPE_LEVEL_HIGH>,
Wrong values have been introduced for interrupts property. Fix those ones, and correct the mapping of context banks to irq number. Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform") Signed-off-by: Parikshit Pareek <quic_ppareek@quicinc.com> --- arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)