From: David Lechner <[email protected]>
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.
Example:
irq_set_irqchip_state(irq, IRQCHIP_STATE_PENDING, true);
Signed-off-by: David Lechner <[email protected]>
Signed-off-by: Suman Anna <[email protected]>
Signed-off-by: Grzegorz Jaszczyk <[email protected]>
Reviewed-by: Lee Jones <[email protected]>
---
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 <[email protected]>
*/
+#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);
+ mask = BIT(data->hwirq % 32);
+
+ srsr = pruss_intc_read_reg(intc, reg);
+
+ *state = !!(srsr & mask);
+
+ return 0;
+}
+
+static int pruss_intc_irq_set_irqchip_state(struct irq_data *data,
+ enum irqchip_irq_state which,
+ bool state)
+{
+ struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
+
+ if (which != IRQCHIP_STATE_PENDING)
+ return -EINVAL;
+
+ if (state)
+ pruss_intc_write_reg(intc, PRU_INTC_SISR, data->hwirq);
+ else
+ pruss_intc_write_reg(intc, PRU_INTC_SICR, data->hwirq);
+
+ return 0;
+}
+
static struct irq_chip pruss_irqchip = {
.name = "pruss-intc",
.irq_ack = pruss_intc_irq_ack,
@@ -152,6 +189,8 @@ static struct irq_chip pruss_irqchip = {
.irq_unmask = pruss_intc_irq_unmask,
.irq_request_resources = pruss_intc_irq_reqres,
.irq_release_resources = pruss_intc_irq_relres,
+ .irq_get_irqchip_state = pruss_intc_irq_get_irqchip_state,
+ .irq_set_irqchip_state = pruss_intc_irq_set_irqchip_state,
};
static int pruss_intc_irq_domain_map(struct irq_domain *d, unsigned int virq,
--
2.7.4
On 2020-07-02 15:17, Grzegorz Jaszczyk wrote:
> From: David Lechner <[email protected]>
>
> 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?
>
> 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.
And from what I have understood of the previous patches, this can't
be the case. What didi I miss?
>
> Signed-off-by: David Lechner <[email protected]>
> Signed-off-by: Suman Anna <[email protected]>
> Signed-off-by: Grzegorz Jaszczyk <[email protected]>
> Reviewed-by: Lee Jones <[email protected]>
> ---
> 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 <[email protected]>
> */
>
> +#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?
> + mask = BIT(data->hwirq % 32);
> +
> + srsr = pruss_intc_read_reg(intc, reg);
> +
> + *state = !!(srsr & mask);
> +
> + return 0;
> +}
> +
> +static int pruss_intc_irq_set_irqchip_state(struct irq_data *data,
> + enum irqchip_irq_state which,
> + bool state)
> +{
> + struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
> +
> + if (which != IRQCHIP_STATE_PENDING)
> + return -EINVAL;
> +
> + if (state)
> + pruss_intc_write_reg(intc, PRU_INTC_SISR, data->hwirq);
> + else
> + pruss_intc_write_reg(intc, PRU_INTC_SICR, data->hwirq);
> +
> + return 0;
> +}
> +
> static struct irq_chip pruss_irqchip = {
> .name = "pruss-intc",
> .irq_ack = pruss_intc_irq_ack,
> @@ -152,6 +189,8 @@ static struct irq_chip pruss_irqchip = {
> .irq_unmask = pruss_intc_irq_unmask,
> .irq_request_resources = pruss_intc_irq_reqres,
> .irq_release_resources = pruss_intc_irq_relres,
> + .irq_get_irqchip_state = pruss_intc_irq_get_irqchip_state,
> + .irq_set_irqchip_state = pruss_intc_irq_set_irqchip_state,
> };
>
> static int pruss_intc_irq_domain_map(struct irq_domain *d, unsigned
> int virq,
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On Thu, 2 Jul 2020 at 19:54, Marc Zyngier <[email protected]> wrote:
>
> On 2020-07-02 15:17, Grzegorz Jaszczyk wrote:
> > From: David Lechner <[email protected]>
> >
> > 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).
>
> >
> > Signed-off-by: David Lechner <[email protected]>
> > Signed-off-by: Suman Anna <[email protected]>
> > Signed-off-by: Grzegorz Jaszczyk <[email protected]>
> > Reviewed-by: Lee Jones <[email protected]>
> > ---
> > 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 <[email protected]>
> > */
> >
> > +#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
On 7/3/20 12:04 PM, Grzegorz Jaszczyk wrote:
> On Thu, 2 Jul 2020 at 19:54, Marc Zyngier <[email protected]> wrote:
>>
>> On 2020-07-02 15:17, Grzegorz Jaszczyk wrote:
>>> From: David Lechner <[email protected]>
>>>
>>> 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 <[email protected]>
>>> Signed-off-by: Suman Anna <[email protected]>
>>> Signed-off-by: Grzegorz Jaszczyk <[email protected]>
>>> Reviewed-by: Lee Jones <[email protected]>
>>> ---
>>> 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 <[email protected]>
>>> */
>>>
>>> +#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
>