| Message ID | 536CD01F.2070506@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Friday 09 May 2014 08:54 AM, Nishanth Menon wrote: > On 05/08/2014 11:22 PM, Joel Fernandes wrote: >> On Thu, May 8, 2014 at 7:25 PM, Santosh Shilimkar >> <santosh.shilimkar@ti.com> wrote: > [...] >> Ok, thanks for pointing to the post. >> > > > Yep - thanks Santosh for clarifying this. Now, we still have the > issues that I pointed out in [1] - without resolving which, we should > not enable crossbar for dra74x/72x. > > A. taking example of PMU > interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH> > this wont work. instead the crossbar driver needs some sort of a hint > to know that it should not map these on crossbar register instead > assign GIC mapping directly. > > I propose doing the following > #define GIC_CROSSBAR_PASSTHROUGH(irq_no) ((irq_no) | (0x1 << 31)) > > and dts will define the following: > interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(131) IRQ_TYPE_LEVEL_HIGH> > > This will also work for the other cases (B.2, B.3) > > For B.2: L3_APP_IRQ: > instead of: > interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH> > we do: > interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(10) IRQ_TYPE_LEVEL_HIGH> > > For B.3: NMI > interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(133) IRQ_TYPE_LEVEL_HIGH> > We can't do add a flag to generic interrupt controller flags since its very specific to cross-bar. > xlate is easy -> > > diff --git a/drivers/irqchip/irq-crossbar.c > b/drivers/irqchip/irq-crossbar.c > index de021638..fd09ab4 100644 > --- a/drivers/irqchip/irq-crossbar.c > +++ b/drivers/irqchip/irq-crossbar.c > @@ -112,6 +112,10 @@ static int crossbar_domain_xlate(struct > irq_domain *d, > { > unsigned long ret; > > + /* Check to see if direct GIC mapping is required */ > + if (intspec[1] & BIT(31)) > + return intspec[1] & ~BIT[31]; > + > ret = get_prev_map_irq(intspec[1]); > if (!IS_ERR_VALUE(ret)) > goto found; > > But then, crossbar_domain_map and crossbar_domain_unmap need hints as > well to know that there is no corresponding crossbar registers. > Have'nt thought through that yet. Looking to hear about opinions here. > > May be we need additional property like reserved to take care of 1:1 map. ti,irqs-direct-map = <131 132>; -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Nishanth, On 05/09/2014 07:54 AM, Nishanth Menon wrote: [..] > Yep - thanks Santosh for clarifying this. Now, we still have the > issues that I pointed out in [1] - without resolving which, we should > not enable crossbar for dra74x/72x. > > A. taking example of PMU > interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH> > this wont work. instead the crossbar driver needs some sort of a hint > to know that it should not map these on crossbar register instead > assign GIC mapping directly. > > I propose doing the following > #define GIC_CROSSBAR_PASSTHROUGH(irq_no) ((irq_no) | (0x1 << 31)) > > and dts will define the following: > interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(131) IRQ_TYPE_LEVEL_HIGH> I would pick something smaller like GIC_SKIP_CROSSBAR. > This will also work for the other cases (B.2, B.3) > > For B.2: L3_APP_IRQ: > instead of: > interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH> > we do: > interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(10) IRQ_TYPE_LEVEL_HIGH> > > For B.3: NMI > interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(133) IRQ_TYPE_LEVEL_HIGH> > > xlate is easy -> > > diff --git a/drivers/irqchip/irq-crossbar.c > b/drivers/irqchip/irq-crossbar.c > index de021638..fd09ab4 100644 > --- a/drivers/irqchip/irq-crossbar.c > +++ b/drivers/irqchip/irq-crossbar.c > @@ -112,6 +112,10 @@ static int crossbar_domain_xlate(struct > irq_domain *d, > { > unsigned long ret; > > + /* Check to see if direct GIC mapping is required */ > + if (intspec[1] & BIT(31)) > + return intspec[1] & ~BIT[31]; > + > ret = get_prev_map_irq(intspec[1]); > if (!IS_ERR_VALUE(ret)) Sounds good, one problem I see here though is once you do the xlate, the information that the IRQ number is GIC cross bar is lost because you are 0'ing bit 31. Then how will map/unmap decide if it needs to be crossbar mapped/unmapped or GIC? Perhaps, the info in bit 31 should be stored somewhere and reused later during map time, or I am missing something. thanks, -Joel -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/09/2014 08:27 AM, Santosh Shilimkar wrote: > On Friday 09 May 2014 08:54 AM, Nishanth Menon wrote: >> On 05/08/2014 11:22 PM, Joel Fernandes wrote: >>> On Thu, May 8, 2014 at 7:25 PM, Santosh Shilimkar >>> <santosh.shilimkar@ti.com> wrote: >> [...] >>> Ok, thanks for pointing to the post. >>> >> >> >> Yep - thanks Santosh for clarifying this. Now, we still have the >> issues that I pointed out in [1] - without resolving which, we should >> not enable crossbar for dra74x/72x. >> >> A. taking example of PMU >> interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH> >> this wont work. instead the crossbar driver needs some sort of a hint >> to know that it should not map these on crossbar register instead >> assign GIC mapping directly. >> >> I propose doing the following >> #define GIC_CROSSBAR_PASSTHROUGH(irq_no) ((irq_no) | (0x1 << 31)) >> >> and dts will define the following: >> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(131) IRQ_TYPE_LEVEL_HIGH> >> >> This will also work for the other cases (B.2, B.3) >> >> For B.2: L3_APP_IRQ: >> instead of: >> interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH> >> we do: >> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(10) IRQ_TYPE_LEVEL_HIGH> >> >> For B.3: NMI >> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(133) IRQ_TYPE_LEVEL_HIGH> >> > We can't do add a flag to generic interrupt controller flags since its > very specific to cross-bar. > >> xlate is easy -> >> >> diff --git a/drivers/irqchip/irq-crossbar.c >> b/drivers/irqchip/irq-crossbar.c >> index de021638..fd09ab4 100644 >> --- a/drivers/irqchip/irq-crossbar.c >> +++ b/drivers/irqchip/irq-crossbar.c >> @@ -112,6 +112,10 @@ static int crossbar_domain_xlate(struct >> irq_domain *d, >> { >> unsigned long ret; >> >> + /* Check to see if direct GIC mapping is required */ >> + if (intspec[1] & BIT(31)) >> + return intspec[1] & ~BIT[31]; >> + >> ret = get_prev_map_irq(intspec[1]); >> if (!IS_ERR_VALUE(ret)) >> goto found; >> >> But then, crossbar_domain_map and crossbar_domain_unmap need hints as >> well to know that there is no corresponding crossbar registers. >> Have'nt thought through that yet. Looking to hear about opinions here. >> >> > May be we need additional property like reserved to take care of 1:1 > map. > > ti,irqs-direct-map = <131 132>; > We already have equivalents for these -> reserved and skip. Problem is how does crossbar driver know the difference between direct maps and crossbar value? 6 is one of those reserved ones. dts for a device says: interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH> Now, xlate gets intspec[1] = 6. 6 is valid crossbar number PRM_IRQ_MPU, however GIC 6 is mapped to WD_TIMER_MPU_C1_IRQ_WARN -> you need to be able to get a hint that this is direct mapping dts intended. in the "6" example: How do i get PRM_IRQ_MPU? interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH> How do I get WD_TIMER_MPU_C1_IRQ_WARN? interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH> ????? - that wont work as crossbar driver thinks it is crossbar 6 (PRM_IRQ_MPU)
On 05/09/2014 08:36 AM, Joel Fernandes wrote: > Hi Nishanth, > > On 05/09/2014 07:54 AM, Nishanth Menon wrote: > [..] >> Yep - thanks Santosh for clarifying this. Now, we still have the >> issues that I pointed out in [1] - without resolving which, we should >> not enable crossbar for dra74x/72x. >> >> A. taking example of PMU >> interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH> >> this wont work. instead the crossbar driver needs some sort of a hint >> to know that it should not map these on crossbar register instead >> assign GIC mapping directly. >> >> I propose doing the following >> #define GIC_CROSSBAR_PASSTHROUGH(irq_no) ((irq_no) | (0x1 << 31)) >> >> and dts will define the following: >> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(131) IRQ_TYPE_LEVEL_HIGH> > > I would pick something smaller like GIC_SKIP_CROSSBAR. > >> This will also work for the other cases (B.2, B.3) >> >> For B.2: L3_APP_IRQ: >> instead of: >> interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH> >> we do: >> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(10) IRQ_TYPE_LEVEL_HIGH> >> >> For B.3: NMI >> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(133) IRQ_TYPE_LEVEL_HIGH> >> >> xlate is easy -> >> >> diff --git a/drivers/irqchip/irq-crossbar.c >> b/drivers/irqchip/irq-crossbar.c >> index de021638..fd09ab4 100644 >> --- a/drivers/irqchip/irq-crossbar.c >> +++ b/drivers/irqchip/irq-crossbar.c >> @@ -112,6 +112,10 @@ static int crossbar_domain_xlate(struct >> irq_domain *d, >> { >> unsigned long ret; >> >> + /* Check to see if direct GIC mapping is required */ >> + if (intspec[1] & BIT(31)) >> + return intspec[1] & ~BIT[31]; >> + >> ret = get_prev_map_irq(intspec[1]); >> if (!IS_ERR_VALUE(ret)) > > Sounds good, one problem I see here though is once you do the xlate, the > information that the IRQ number is GIC cross bar is lost because you are > 0'ing bit 31. Then how will map/unmap decide if it needs to be crossbar > mapped/unmapped or GIC? > Correcting myself here, I meant "IRQ number is GIC cross bar skipped". -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/09/2014 08:36 AM, Joel Fernandes wrote: > Hi Nishanth, > > On 05/09/2014 07:54 AM, Nishanth Menon wrote: > [..] >> Yep - thanks Santosh for clarifying this. Now, we still have the >> issues that I pointed out in [1] - without resolving which, we should >> not enable crossbar for dra74x/72x. >> >> A. taking example of PMU >> interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH> >> this wont work. instead the crossbar driver needs some sort of a hint >> to know that it should not map these on crossbar register instead >> assign GIC mapping directly. >> >> I propose doing the following >> #define GIC_CROSSBAR_PASSTHROUGH(irq_no) ((irq_no) | (0x1 << 31)) >> >> and dts will define the following: >> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(131) IRQ_TYPE_LEVEL_HIGH> > > I would pick something smaller like GIC_SKIP_CROSSBAR. > >> This will also work for the other cases (B.2, B.3) >> >> For B.2: L3_APP_IRQ: >> instead of: >> interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH> >> we do: >> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(10) IRQ_TYPE_LEVEL_HIGH> >> >> For B.3: NMI >> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(133) IRQ_TYPE_LEVEL_HIGH> >> >> xlate is easy -> >> >> diff --git a/drivers/irqchip/irq-crossbar.c >> b/drivers/irqchip/irq-crossbar.c >> index de021638..fd09ab4 100644 >> --- a/drivers/irqchip/irq-crossbar.c >> +++ b/drivers/irqchip/irq-crossbar.c >> @@ -112,6 +112,10 @@ static int crossbar_domain_xlate(struct >> irq_domain *d, >> { >> unsigned long ret; >> >> + /* Check to see if direct GIC mapping is required */ >> + if (intspec[1] & BIT(31)) >> + return intspec[1] & ~BIT[31]; >> + >> ret = get_prev_map_irq(intspec[1]); >> if (!IS_ERR_VALUE(ret)) > > Sounds good, one problem I see here though is once you do the xlate, the > information that the IRQ number is GIC cross bar is lost because you are > 0'ing bit 31. Then how will map/unmap decide if it needs to be crossbar > mapped/unmapped or GIC? > > Perhaps, the info in bit 31 should be stored somewhere and reused later > during map time, or I am missing something. no, you did not miss anything -> I did mention in my mail precisely that "But then, crossbar_domain_map and crossbar_domain_unmap need hints as well to know that there is no corresponding crossbar registers. Have'nt thought through that yet." Lets discuss hardware description problem(dts) first and then solve the driver problem next.
On 05/09/2014 08:27 AM, Santosh Shilimkar wrote: > On Friday 09 May 2014 08:54 AM, Nishanth Menon wrote: >> On 05/08/2014 11:22 PM, Joel Fernandes wrote: >>> On Thu, May 8, 2014 at 7:25 PM, Santosh Shilimkar >>> <santosh.shilimkar@ti.com> wrote: >> [...] >>> Ok, thanks for pointing to the post. >>> >> >> >> Yep - thanks Santosh for clarifying this. Now, we still have the >> issues that I pointed out in [1] - without resolving which, we should >> not enable crossbar for dra74x/72x. >> >> A. taking example of PMU >> interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH> >> this wont work. instead the crossbar driver needs some sort of a hint >> to know that it should not map these on crossbar register instead >> assign GIC mapping directly. >> >> I propose doing the following >> #define GIC_CROSSBAR_PASSTHROUGH(irq_no) ((irq_no) | (0x1 << 31)) >> >> and dts will define the following: >> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(131) IRQ_TYPE_LEVEL_HIGH> >> >> This will also work for the other cases (B.2, B.3) >> >> For B.2: L3_APP_IRQ: >> instead of: >> interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH> >> we do: >> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(10) IRQ_TYPE_LEVEL_HIGH> >> >> For B.3: NMI >> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(133) IRQ_TYPE_LEVEL_HIGH> >> > We can't do add a flag to generic interrupt controller flags since its > very specific to cross-bar. > >> xlate is easy -> >> >> diff --git a/drivers/irqchip/irq-crossbar.c >> b/drivers/irqchip/irq-crossbar.c >> index de021638..fd09ab4 100644 >> --- a/drivers/irqchip/irq-crossbar.c >> +++ b/drivers/irqchip/irq-crossbar.c >> @@ -112,6 +112,10 @@ static int crossbar_domain_xlate(struct >> irq_domain *d, >> { >> unsigned long ret; >> >> + /* Check to see if direct GIC mapping is required */ >> + if (intspec[1] & BIT(31)) >> + return intspec[1] & ~BIT[31]; >> + >> ret = get_prev_map_irq(intspec[1]); >> if (!IS_ERR_VALUE(ret)) >> goto found; >> >> But then, crossbar_domain_map and crossbar_domain_unmap need hints as >> well to know that there is no corresponding crossbar registers. >> Have'nt thought through that yet. Looking to hear about opinions here. >> >> > May be we need additional property like reserved to take care of 1:1 > map. > > ti,irqs-direct-map = <131 132>; > That wont work for cases where the cross bar has hardware bugs as Nishanth pointed in his example. For such cases, you need to differentiate between a "direct-map" GIC IRQ and a regular working logical cross-bar number. For example, what if IRQ no 10 has a crossbar bug? Does this mean you will also make crossbar number (not IRQ no, logical cross bar no) also suffer? The way to fix this is like Nishanth pointed by introducing some encoding or flag: For someone using the logical cross bar, they would say: interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH> and in the same dts, for someone else who is using the direct-mapped buggy IRQ 10, they would use the the PASSTHROUGH flag: interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(10) IRQ_TYPE_LEVEL_HIGH> In this case, a "direct-map" list as you suggested doesn't work. thanks, -Joel -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 09 May 2014 09:36 AM, Nishanth Menon wrote: > On 05/09/2014 08:27 AM, Santosh Shilimkar wrote: >> On Friday 09 May 2014 08:54 AM, Nishanth Menon wrote: >>> On 05/08/2014 11:22 PM, Joel Fernandes wrote: >>>> On Thu, May 8, 2014 at 7:25 PM, Santosh Shilimkar >>>> <santosh.shilimkar@ti.com> wrote: >>> [...] >>>> Ok, thanks for pointing to the post. >>>> >>> >>> >>> Yep - thanks Santosh for clarifying this. Now, we still have the >>> issues that I pointed out in [1] - without resolving which, we should >>> not enable crossbar for dra74x/72x. >>> >>> A. taking example of PMU >>> interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH> >>> this wont work. instead the crossbar driver needs some sort of a hint >>> to know that it should not map these on crossbar register instead >>> assign GIC mapping directly. >>> >>> I propose doing the following >>> #define GIC_CROSSBAR_PASSTHROUGH(irq_no) ((irq_no) | (0x1 << 31)) >>> >>> and dts will define the following: >>> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(131) IRQ_TYPE_LEVEL_HIGH> >>> >>> This will also work for the other cases (B.2, B.3) >>> >>> For B.2: L3_APP_IRQ: >>> instead of: >>> interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH> >>> we do: >>> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(10) IRQ_TYPE_LEVEL_HIGH> >>> >>> For B.3: NMI >>> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(133) IRQ_TYPE_LEVEL_HIGH> >>> >> We can't do add a flag to generic interrupt controller flags since its >> very specific to cross-bar. >> >>> xlate is easy -> >>> >>> diff --git a/drivers/irqchip/irq-crossbar.c >>> b/drivers/irqchip/irq-crossbar.c >>> index de021638..fd09ab4 100644 >>> --- a/drivers/irqchip/irq-crossbar.c >>> +++ b/drivers/irqchip/irq-crossbar.c >>> @@ -112,6 +112,10 @@ static int crossbar_domain_xlate(struct >>> irq_domain *d, >>> { >>> unsigned long ret; >>> >>> + /* Check to see if direct GIC mapping is required */ >>> + if (intspec[1] & BIT(31)) >>> + return intspec[1] & ~BIT[31]; >>> + >>> ret = get_prev_map_irq(intspec[1]); >>> if (!IS_ERR_VALUE(ret)) >>> goto found; >>> >>> But then, crossbar_domain_map and crossbar_domain_unmap need hints as >>> well to know that there is no corresponding crossbar registers. >>> Have'nt thought through that yet. Looking to hear about opinions here. >>> >>> >> May be we need additional property like reserved to take care of 1:1 >> map. >> >> ti,irqs-direct-map = <131 132>; >> > We already have equivalents for these -> reserved and skip. Problem is > how does crossbar driver know the difference between direct maps and > crossbar value? > > 6 is one of those reserved ones. dts for a device says: > interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH> > > > Now, xlate gets intspec[1] = 6. 6 is valid crossbar number > PRM_IRQ_MPU, however GIC 6 is mapped to WD_TIMER_MPU_C1_IRQ_WARN -> > you need to be able to get a hint that this is direct mapping dts > intended. > > in the "6" example: > > How do i get PRM_IRQ_MPU? > interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH> > > How do I get WD_TIMER_MPU_C1_IRQ_WARN? > interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH> ????? - that wont work as > crossbar driver thinks it is crossbar 6 (PRM_IRQ_MPU) > Looks like I am missing something. Is the issue because of SPI offset (32) which makes above confusion ? -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/09/2014 08:45 AM, Santosh Shilimkar wrote: > On Friday 09 May 2014 09:36 AM, Nishanth Menon wrote: >> On 05/09/2014 08:27 AM, Santosh Shilimkar wrote: >>> On Friday 09 May 2014 08:54 AM, Nishanth Menon wrote: >>>> On 05/08/2014 11:22 PM, Joel Fernandes wrote: >>>>> On Thu, May 8, 2014 at 7:25 PM, Santosh Shilimkar >>>>> <santosh.shilimkar@ti.com> wrote: >>>> [...] >>>>> Ok, thanks for pointing to the post. >>>>> >>>> >>>> >>>> Yep - thanks Santosh for clarifying this. Now, we still have the >>>> issues that I pointed out in [1] - without resolving which, we should >>>> not enable crossbar for dra74x/72x. >>>> >>>> A. taking example of PMU >>>> interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH> >>>> this wont work. instead the crossbar driver needs some sort of a hint >>>> to know that it should not map these on crossbar register instead >>>> assign GIC mapping directly. >>>> >>>> I propose doing the following >>>> #define GIC_CROSSBAR_PASSTHROUGH(irq_no) ((irq_no) | (0x1 << 31)) >>>> >>>> and dts will define the following: >>>> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(131) IRQ_TYPE_LEVEL_HIGH> >>>> >>>> This will also work for the other cases (B.2, B.3) >>>> >>>> For B.2: L3_APP_IRQ: >>>> instead of: >>>> interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH> >>>> we do: >>>> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(10) IRQ_TYPE_LEVEL_HIGH> >>>> >>>> For B.3: NMI >>>> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(133) IRQ_TYPE_LEVEL_HIGH> >>>> >>> We can't do add a flag to generic interrupt controller flags since its >>> very specific to cross-bar. >>> >>>> xlate is easy -> >>>> >>>> diff --git a/drivers/irqchip/irq-crossbar.c >>>> b/drivers/irqchip/irq-crossbar.c >>>> index de021638..fd09ab4 100644 >>>> --- a/drivers/irqchip/irq-crossbar.c >>>> +++ b/drivers/irqchip/irq-crossbar.c >>>> @@ -112,6 +112,10 @@ static int crossbar_domain_xlate(struct >>>> irq_domain *d, >>>> { >>>> unsigned long ret; >>>> >>>> + /* Check to see if direct GIC mapping is required */ >>>> + if (intspec[1] & BIT(31)) >>>> + return intspec[1] & ~BIT[31]; >>>> + >>>> ret = get_prev_map_irq(intspec[1]); >>>> if (!IS_ERR_VALUE(ret)) >>>> goto found; >>>> >>>> But then, crossbar_domain_map and crossbar_domain_unmap need hints as >>>> well to know that there is no corresponding crossbar registers. >>>> Have'nt thought through that yet. Looking to hear about opinions here. >>>> >>>> >>> May be we need additional property like reserved to take care of 1:1 >>> map. >>> >>> ti,irqs-direct-map = <131 132>; >>> >> We already have equivalents for these -> reserved and skip. Problem is >> how does crossbar driver know the difference between direct maps and >> crossbar value? >> >> 6 is one of those reserved ones. dts for a device says: >> interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH> >> >> >> Now, xlate gets intspec[1] = 6. 6 is valid crossbar number >> PRM_IRQ_MPU, however GIC 6 is mapped to WD_TIMER_MPU_C1_IRQ_WARN -> >> you need to be able to get a hint that this is direct mapping dts >> intended. >> >> in the "6" example: >> >> How do i get PRM_IRQ_MPU? >> interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH> >> >> How do I get WD_TIMER_MPU_C1_IRQ_WARN? >> interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH> ????? - that wont work as >> crossbar driver thinks it is crossbar 6 (PRM_IRQ_MPU) >> > Looks like I am missing something. Is the issue because of SPI offset (32) > which makes above confusion ? The way we modelled crossbar is that the irqchip is GIC and routable mapper is crossbar - which makes sense. now for every "interrupts" description we try to find a map using the crossbar driver as the irq framework rightly identifies that peripheral interrupts are routable and crossbar driver is the guy who knows how to map these to a proper GIC interrupt number. So far, we are good. Now the trouble starts when our hardware description in dts assumes that every peripheral interrupt is a routable interrupt - as this thread describes - that is NOT the case. Now the question is how do you differentiate? interrupts = <GIC_SPI Number Level> GIC_SPI is correct since we are attempting to describe the SPI interrupt (offset what ever it is, is a NOP from conceptual discussion). Level is fine as well. Number: what does this indicate? crossbar number is what we have assumed so far. however, then you loose the ability to describe interrupts on GIC SPI which dont have crossbar interrupts. Question is how do we differentiate between the two?
On 05/09/2014 09:00 AM, Nishanth Menon wrote: > On 05/09/2014 08:45 AM, Santosh Shilimkar wrote: >> On Friday 09 May 2014 09:36 AM, Nishanth Menon wrote: >>> On 05/09/2014 08:27 AM, Santosh Shilimkar wrote: >>>> On Friday 09 May 2014 08:54 AM, Nishanth Menon wrote: >>>>> On 05/08/2014 11:22 PM, Joel Fernandes wrote: >>>>>> On Thu, May 8, 2014 at 7:25 PM, Santosh Shilimkar >>>>>> <santosh.shilimkar@ti.com> wrote: >>>>> [...] >>>>>> Ok, thanks for pointing to the post. >>>>>> >>>>> >>>>> >>>>> Yep - thanks Santosh for clarifying this. Now, we still have the >>>>> issues that I pointed out in [1] - without resolving which, we should >>>>> not enable crossbar for dra74x/72x. >>>>> >>>>> A. taking example of PMU >>>>> interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH> >>>>> this wont work. instead the crossbar driver needs some sort of a hint >>>>> to know that it should not map these on crossbar register instead >>>>> assign GIC mapping directly. >>>>> >>>>> I propose doing the following >>>>> #define GIC_CROSSBAR_PASSTHROUGH(irq_no) ((irq_no) | (0x1 << 31)) >>>>> >>>>> and dts will define the following: >>>>> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(131) IRQ_TYPE_LEVEL_HIGH> >>>>> >>>>> This will also work for the other cases (B.2, B.3) >>>>> >>>>> For B.2: L3_APP_IRQ: >>>>> instead of: >>>>> interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH> >>>>> we do: >>>>> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(10) IRQ_TYPE_LEVEL_HIGH> >>>>> >>>>> For B.3: NMI >>>>> interrupts = <GIC_SPI GIC_CROSSBAR_PASSTHROUGH(133) IRQ_TYPE_LEVEL_HIGH> >>>>> >>>> We can't do add a flag to generic interrupt controller flags since its >>>> very specific to cross-bar. >>>> >>>>> xlate is easy -> >>>>> >>>>> diff --git a/drivers/irqchip/irq-crossbar.c >>>>> b/drivers/irqchip/irq-crossbar.c >>>>> index de021638..fd09ab4 100644 >>>>> --- a/drivers/irqchip/irq-crossbar.c >>>>> +++ b/drivers/irqchip/irq-crossbar.c >>>>> @@ -112,6 +112,10 @@ static int crossbar_domain_xlate(struct >>>>> irq_domain *d, >>>>> { >>>>> unsigned long ret; >>>>> >>>>> + /* Check to see if direct GIC mapping is required */ >>>>> + if (intspec[1] & BIT(31)) >>>>> + return intspec[1] & ~BIT[31]; >>>>> + >>>>> ret = get_prev_map_irq(intspec[1]); >>>>> if (!IS_ERR_VALUE(ret)) >>>>> goto found; >>>>> >>>>> But then, crossbar_domain_map and crossbar_domain_unmap need hints as >>>>> well to know that there is no corresponding crossbar registers. >>>>> Have'nt thought through that yet. Looking to hear about opinions here. >>>>> >>>>> >>>> May be we need additional property like reserved to take care of 1:1 >>>> map. >>>> >>>> ti,irqs-direct-map = <131 132>; >>>> >>> We already have equivalents for these -> reserved and skip. Problem is >>> how does crossbar driver know the difference between direct maps and >>> crossbar value? >>> >>> 6 is one of those reserved ones. dts for a device says: >>> interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH> >>> >>> >>> Now, xlate gets intspec[1] = 6. 6 is valid crossbar number >>> PRM_IRQ_MPU, however GIC 6 is mapped to WD_TIMER_MPU_C1_IRQ_WARN -> >>> you need to be able to get a hint that this is direct mapping dts >>> intended. >>> >>> in the "6" example: >>> >>> How do i get PRM_IRQ_MPU? >>> interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH> >>> >>> How do I get WD_TIMER_MPU_C1_IRQ_WARN? >>> interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH> ????? - that wont work as >>> crossbar driver thinks it is crossbar 6 (PRM_IRQ_MPU) >>> >> Looks like I am missing something. Is the issue because of SPI offset (32) >> which makes above confusion ? > > The way we modelled crossbar is that the irqchip is GIC and routable > mapper is crossbar - which makes sense. now for every "interrupts" > description we try to find a map using the crossbar driver as the irq > framework rightly identifies that peripheral interrupts are routable > and crossbar driver is the guy who knows how to map these to a proper > GIC interrupt number. So far, we are good. > > Now the trouble starts when our hardware description in dts assumes > that every peripheral interrupt is a routable interrupt - as this > thread describes - that is NOT the case. > > Now the question is how do you differentiate? > > interrupts = <GIC_SPI Number Level> > > GIC_SPI is correct since we are attempting to describe the SPI > interrupt (offset what ever it is, is a NOP from conceptual discussion). > > Level is fine as well. > > Number: what does this indicate? crossbar number is what we have > assumed so far. however, then you loose the ability to describe > interrupts on GIC SPI which dont have crossbar interrupts. > > Question is how do we differentiate between the two? > IMO even the direct-mapped ones can go come from a list, trouble is with the buggy hardware hard-coded mappings which map some random crossbar to some random GIC and can't be changed. Only way to handle this is with a sort of an associative map in DT, not a list. But that's far more overkill than anyone could imagine, and the suggestion to set BIT 31 is cleaner without needing any lists at all, while hiding all the complexity within the crossbar driver. After xlate, the IRQ number is corrected, so the flag disappears, and the info that its DIRECT can be stored to retrieved later during map/unmap. thanks, -Joel -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 09 May 2014 10:00 AM, Nishanth Menon wrote: > On 05/09/2014 08:45 AM, Santosh Shilimkar wrote: >> On Friday 09 May 2014 09:36 AM, Nishanth Menon wrote: [..] >> Looks like I am missing something. Is the issue because of SPI offset (32) >> which makes above confusion ? > > The way we modelled crossbar is that the irqchip is GIC and routable > mapper is crossbar - which makes sense. now for every "interrupts" > description we try to find a map using the crossbar driver as the irq > framework rightly identifies that peripheral interrupts are routable > and crossbar driver is the guy who knows how to map these to a proper > GIC interrupt number. So far, we are good. > > Now the trouble starts when our hardware description in dts assumes > that every peripheral interrupt is a routable interrupt - as this > thread describes - that is NOT the case. > > Now the question is how do you differentiate? > > interrupts = <GIC_SPI Number Level> > > GIC_SPI is correct since we are attempting to describe the SPI > interrupt (offset what ever it is, is a NOP from conceptual discussion). > > Level is fine as well. > > Number: what does this indicate? crossbar number is what we have > assumed so far. however, then you loose the ability to describe > interrupts on GIC SPI which dont have crossbar interrupts. > > Question is how do we differentiate between the two? > Updating the thread about an off-list discussion on $subject. Nishant understood the how he was indirectly changing the meaning of IRQ bindings and why its a bad idea. The point which didn't came out clearly on the list was also the limited set of route-able IRQ registers at cross-bar which is utter non-sense and broken hardware. And since we are patching up the cross-bar hardware bugs, it should be done such a way that the cross-bar device tree bindings reflect that. Here is the way forward we decided... Add additional/optional properties to cross-bar binding so that you can represent IRQs which can't be routed by cross-bar. Something like 'MAX_ROUTABLE_IRQS'. Then on the cleint modules which has the shortcoming, you just add MAX_ROUTABLE_IRQS to those IRQs. That way cross-bar driver fails to route them and just returns the offset IRQline thinking it is already hard wired. As a proof of concept I suggest you to just create a bidnig patch with example usage of such case in the Documentation and send it for review. Copy Device Tree folks and Thomas to see if they are ok with it or can suggest some better way to deal with the issue. Thanks for the patience and explaining the issue repeatedly. regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c index de021638..fd09ab4 100644 --- a/drivers/irqchip/irq-crossbar.c +++ b/drivers/irqchip/irq-crossbar.c @@ -112,6 +112,10 @@ static int crossbar_domain_xlate(struct irq_domain *d, { unsigned long ret; + /* Check to see if direct GIC mapping is required */ + if (intspec[1] & BIT(31)) + return intspec[1] & ~BIT[31]; + ret = get_prev_map_irq(intspec[1]); if (!IS_ERR_VALUE(ret))