lkml.org 
[lkml]   [2020]   [Jul]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCHv3 4/6] irqchip/irq-pruss-intc: Implement irq_{get,set}_irqchip_state ops
From
Date
On 7/3/20 12:04 PM, Grzegorz Jaszczyk wrote:
> On Thu, 2 Jul 2020 at 19:54, Marc Zyngier <maz@kernel.org> wrote:
>>
>> On 2020-07-02 15:17, Grzegorz Jaszczyk wrote:
>>> From: David Lechner <david@lechnology.com>
>>>
>>> This implements the irq_get_irqchip_state and irq_set_irqchip_state
>>> callbacks for the TI PRUSS INTC driver. The set callback can be used
>>> by drivers to "kick" a PRU by enabling a PRU system event.
>>
>> "enabling"? That'd be unmasking an interrupt, which isn't what this
>> does. "injecting", maybe?
>
> Yes "injecting" is much better.
>
>>
>>>
>>> Example:
>>> irq_set_irqchip_state(irq, IRQCHIP_STATE_PENDING, true);
>>
>> Nice example.
>>
>> What this example does explain is how you are actually going to kick
>> a PRU via this interface. For that to happen, you'd have to have on
>> the Linux side an interrupt that is actually routed to a PRU.
>
> Correct.
>
>> And from what I have understood of the previous patches, this can't
>> be the case. What didi I miss?
>
> The hwirq's handled by this driver are so called system events in
> PRUSS nomenclature. This driver is responsible for the entire mapping
> of those system events to PRUSS specific channels which are next
> mapped to host_irq (patch #6 https://lkml.org/lkml/2020/7/2/612).
> There are 8 host_irqs that are routed to the main cpu (running Linux)
> and they are called host_intr0..host_intr7 (were seen in previous
> patches of this series). But there are other "host_interrupts" that
> are routed not to the main CPU but to PRU cores and this driver is
> responsible for creating proper mapping (system
> event/channel/host_irq) for them, and allowing to kick PRU via the
> introduced interface.
>
> It is worth noting that the PRUSS is quite flexible and allows various
> things e.g.:
> - map any of 160/64 internal system events to any of the 20/10 channels
> - map any of the 20/10 channels to any of the 20/10 host interrupts.
>
> So e.g. it is possible to map e.g. system event 17 to the main CPU
> (through e.g. channel 1 which is the next map to e.g. host_intr0). Or
> (exclusively) map the same system event 17 to PRU core (through e.g.
> channel 1 which is the next map to PRU0).

Grzegorz has explained in detail, the short summary is we trigger a
system event, and the mapping for that event ensures the interrupt is
routed at the desired processor.

regards
Suman

>
>>
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>> Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>>> Reviewed-by: Lee Jones <lee.jones@linaro.org>
>>> ---
>>> v2->v3:
>>> - Get rid of unnecessary pruss_intc_check_write() and use
>>> pruss_intc_write_reg directly.
>>> v1->v2:
>>> - https://patchwork.kernel.org/patch/11069769/
>>> ---
>>> drivers/irqchip/irq-pruss-intc.c | 43
>>> ++++++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 41 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-pruss-intc.c
>>> b/drivers/irqchip/irq-pruss-intc.c
>>> index 49c936f..19b3d38 100644
>>> --- a/drivers/irqchip/irq-pruss-intc.c
>>> +++ b/drivers/irqchip/irq-pruss-intc.c
>>> @@ -7,6 +7,7 @@
>>> * Suman Anna <s-anna@ti.com>
>>> */
>>>
>>> +#include <linux/interrupt.h>
>>> #include <linux/irq.h>
>>> #include <linux/irqchip/chained_irq.h>
>>> #include <linux/irqdomain.h>
>>> @@ -39,8 +40,7 @@
>>> #define PRU_INTC_HIEISR 0x0034
>>> #define PRU_INTC_HIDISR 0x0038
>>> #define PRU_INTC_GPIR 0x0080
>>> -#define PRU_INTC_SRSR0 0x0200
>>> -#define PRU_INTC_SRSR1 0x0204
>>> +#define PRU_INTC_SRSR(x) (0x0200 + (x) * 4)
>>> #define PRU_INTC_SECR0 0x0280
>>> #define PRU_INTC_SECR1 0x0284
>>> #define PRU_INTC_ESR0 0x0300
>>> @@ -145,6 +145,43 @@ static void pruss_intc_irq_relres(struct irq_data
>>> *data)
>>> module_put(THIS_MODULE);
>>> }
>>>
>>> +static int pruss_intc_irq_get_irqchip_state(struct irq_data *data,
>>> + enum irqchip_irq_state which,
>>> + bool *state)
>>> +{
>>> + struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
>>> + u32 reg, mask, srsr;
>>> +
>>> + if (which != IRQCHIP_STATE_PENDING)
>>> + return -EINVAL;
>>> +
>>> + reg = PRU_INTC_SRSR(data->hwirq / 32);
>>
>> I assume the register file scales as more interrupts are added in the
>> subsequent patch?
>>
> Yes, after I will move part of the next patch to patch #2 as you
> suggested it will stop being confusing.
>
> Thank you,
> Grzegorz
>

\
 
 \ /
  Last update: 2020-07-10 23:07    [W:0.139 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site