Generic DT binding for IPIs

Generic DT binding for IPIs

[Qais Yousef]

Hi,

This is an attempt to revive a discussion on the right list this time 
with all the correct people hopefully on CC.

While trying to upstream a driver, Thomas and Marc Zyngier pointed out 
the need for a generic IPI support in the kernel to allow driver to 
reserve and send ones. Hopefully my latest RFC patch will help to 
clarify what's being done.

     https://lkml.org/lkml/2015/10/13/227

We need a generic DT binding support to accompany that to allow a driver 
to reserve an IPI using this new mechanism.

MarcZ had the following suggestion:

     https://lkml.org/lkml/2015/8/24/628

Which in summary is

	mydevice@f0000000 {
		interrupt-source = <&intc INT_SPEC 2 &inttarg1 &inttarg1>;
	};

	inttarg1: mydevice@f1000000 {
		interrupt-sink = <&intc HWAFFINITY1>;
	};

	inttarg2: cpu@1 {
		interrupt-sink = <&intc HWAFFINITY2>;
	};


interrupt-sink requests to reserve an IPI that it will receive at 
HWAFFINITY cpumask. interrupt-source will not do any reservation. It 
will simply connect an IPI reserved by interrupt-sink to the device that 
will be responsible for generating that IPI. This description should 
allow connecting any 2 devices.
Correct me Marc if I got it wrong please.

I suggested a simplification by assuming that IPIs will only be between 
host OS and a coprocessor which would gives us this form which I think 
is easier to deal with

	coprocessor {
              interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>;
              interrupt-sink = <&intc INT_SPEC CPU_HWAFFINITY>;
	}


interrupt-source here reserves an IPI to be sent from host OS to 
coprocessor at COP_HWAFFINITY. interrupt-sink will reserve an IPI to be 
received by host OS at CPU_HWAFFINITY. Less generic but I don't know how 
important it is for host OS to setup IPIs between 2 external 
coprocessors and whether it should really be doing that.

What do the DT experts think? Any preference or a better suggestion?

I tried to keep this short and simple, please let me know if you need 
more info or if there's anything that needs more clarification.

Thanks,
Qais
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Generic DT binding for IPIs

[Qais Yousef]

Is there anything more I can do to get more attention about this? I 
think Marc's suggestion is more generic and future proof, if I send RFC 
patches for that would this be better?

Thanks,
Qais

On 10/14/2015 11:18 AM, Qais Yousef wrote:
> Hi,
>
> This is an attempt to revive a discussion on the right list this time 
> with all the correct people hopefully on CC.
>
> While trying to upstream a driver, Thomas and Marc Zyngier pointed out 
> the need for a generic IPI support in the kernel to allow driver to 
> reserve and send ones. Hopefully my latest RFC patch will help to 
> clarify what's being done.
>
>     https://lkml.org/lkml/2015/10/13/227
>
> We need a generic DT binding support to accompany that to allow a 
> driver to reserve an IPI using this new mechanism.
>
> MarcZ had the following suggestion:
>
>     https://lkml.org/lkml/2015/8/24/628
>
> Which in summary is
>
>     mydevice@f0000000 {
>         interrupt-source = <&intc INT_SPEC 2 &inttarg1 &inttarg1>;
>     };
>
>     inttarg1: mydevice@f1000000 {
>         interrupt-sink = <&intc HWAFFINITY1>;
>     };
>
>     inttarg2: cpu@1 {
>         interrupt-sink = <&intc HWAFFINITY2>;
>     };
>
>
> interrupt-sink requests to reserve an IPI that it will receive at 
> HWAFFINITY cpumask. interrupt-source will not do any reservation. It 
> will simply connect an IPI reserved by interrupt-sink to the device 
> that will be responsible for generating that IPI. This description 
> should allow connecting any 2 devices.
> Correct me Marc if I got it wrong please.
>
> I suggested a simplification by assuming that IPIs will only be 
> between host OS and a coprocessor which would gives us this form which 
> I think is easier to deal with
>
>     coprocessor {
>              interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>;
>              interrupt-sink = <&intc INT_SPEC CPU_HWAFFINITY>;
>     }
>
>
> interrupt-source here reserves an IPI to be sent from host OS to 
> coprocessor at COP_HWAFFINITY. interrupt-sink will reserve an IPI to 
> be received by host OS at CPU_HWAFFINITY. Less generic but I don't 
> know how important it is for host OS to setup IPIs between 2 external 
> coprocessors and whether it should really be doing that.
>
> What do the DT experts think? Any preference or a better suggestion?
>
> I tried to keep this short and simple, please let me know if you need 
> more info or if there's anything that needs more clarification.
>
> Thanks,
> Qais

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Generic DT binding for IPIs

[Jason Cooper]

On Thu, Oct 22, 2015 at 11:44:16AM +0100, Qais Yousef wrote:
> Is there anything more I can do to get more attention about this? I
> think Marc's suggestion is more generic and future proof, if I send
> RFC patches for that would this be better?

Please do.

thx,

Jason.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Generic DT binding for IPIs

[Qais Yousef]

Hi,

On 10/22/2015 12:55 PM, Jason Cooper wrote:
> On Thu, Oct 22, 2015 at 11:44:16AM +0100, Qais Yousef wrote:
>> Is there anything more I can do to get more attention about this? I
>> think Marc's suggestion is more generic and future proof, if I send
>> RFC patches for that would this be better?
> Please do.

Unfortunately I haven't had a chance to get around writing the patches yet.
I came up with a different description though that I thought maybe worth sharing
to see if there's any opinion about it before the actual work being done.

To summarise, the problem I am trying to solve is that we have a type of
coprocessors which share the interrupt controller with Linux, hence the IPI
mechanism this controller uses. I've been working with Thomas on implementing
a generic API to allocate IPIs for coprocesors and a way for drivers to send
these IPIs [1].

To complement this new API, we need a mechanism to describe this in
device tree so a driver that wants to allocate an IPI can have this done
automatically for it like we handle interrupts.

What I have in mind is:

      coproc {
              ipi-parent = <&gic>;

              ipis = <CPU_VALUE IPI_SPEC>;
              ipi-names = "in";
      };

This will allocate an IPI to go to cpu @CPU_VALUE passing @IPI_SPEC as
parameters to the controller. Which means we need a new ipi-cells to
define how many cells are in ipis property. Note the new ipi-parent too.

I think this is better than interrupt-sink and interrupt-source [2] as we
give the driver the flexibility to give a meaning to what this IPI is.
One thing I found confusing about interrupt-source and interrupt-sink is
from what perspective are we viewing that, host system or firmware..

ipis property also is similar to interrupts, so using it would be easier
(I think).

If we have 2 coprocessors that want to communicate using IPIs that are
managed by the host we use ipi-refs property to refer to IPIs defined in
another node.

      coproc1 {
              ipis = <CPU1>, <CPU2>, <CPU2>;
              ipi-names = "in", "coproc2data", "coproc2ctrl";
      };

      coproc2 {
              ipi-refs = <&coproc1 "in">, <&coproc1 "coproc2data">, <&coproc1 "corpoc2ctrl">;
              ipi-refs-names = "tocorproc1", "indata", "inctrl";
      };

This will cause 3 IPIs to be allocated. One is going to CPU1 and two are
going to CPU2. ipi-names should be similar to how interrupt-names work.

A node can possibly both allocate IPIs and reference other allocated ones.

Thoughts?

Thanks,
Qais

[1]https://lkml.org/lkml/2015/12/8/243
[2]https://lkml.org/lkml/2015/10/14/211


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Generic DT binding for IPIs

[Rob Herring]

On Wed, Dec 9, 2015 at 9:27 AM, Qais Yousef <qais.yousef@imgtec.com> wrote:
> Hi,
>
> On 10/22/2015 12:55 PM, Jason Cooper wrote:
>>
>> On Thu, Oct 22, 2015 at 11:44:16AM +0100, Qais Yousef wrote:
>>>
>>> Is there anything more I can do to get more attention about this? I
>>> think Marc's suggestion is more generic and future proof, if I send
>>> RFC patches for that would this be better?
>>
>> Please do.
>
>
> Unfortunately I haven't had a chance to get around writing the patches yet.
> I came up with a different description though that I thought maybe worth
> sharing
> to see if there's any opinion about it before the actual work being done.

I've not given this too much thought, but here's my initial thoughts.

>
> To summarise, the problem I am trying to solve is that we have a type of
> coprocessors which share the interrupt controller with Linux, hence the IPI
> mechanism this controller uses. I've been working with Thomas on
> implementing
> a generic API to allocate IPIs for coprocesors and a way for drivers to send
> these IPIs [1].
>
> To complement this new API, we need a mechanism to describe this in
> device tree so a driver that wants to allocate an IPI can have this done
> automatically for it like we handle interrupts.
>
> What I have in mind is:
>
>      coproc {
>              ipi-parent = <&gic>;
>
>              ipis = <CPU_VALUE IPI_SPEC>;
>              ipi-names = "in";
>      };
>
> This will allocate an IPI to go to cpu @CPU_VALUE passing @IPI_SPEC as
> parameters to the controller. Which means we need a new ipi-cells to
> define how many cells are in ipis property. Note the new ipi-parent too.

These are still interrupts, so I'd prefer to use or extend the
interrupt binding if possible.

> I think this is better than interrupt-sink and interrupt-source [2] as we
> give the driver the flexibility to give a meaning to what this IPI is.
> One thing I found confusing about interrupt-source and interrupt-sink is
> from what perspective are we viewing that, host system or firmware..

DT is usually from host perspective. But I agree, the naming was still
confusing to me.

> ipis property also is similar to interrupts, so using it would be easier
> (I think).
>
> If we have 2 coprocessors that want to communicate using IPIs that are
> managed by the host we use ipi-refs property to refer to IPIs defined in
> another node.
>
>      coproc1 {
>              ipis = <CPU1>, <CPU2>, <CPU2>;

Don't you need to specify a certain IPI number in addition to which
cpu is the target?

I'm thinking the cpu target could be part of the interrupts property
flags field or something.

>              ipi-names = "in", "coproc2data", "coproc2ctrl";

-names should be optional in general. So define something that works
without them.

>      };
>
>      coproc2 {
>              ipi-refs = <&coproc1 "in">, <&coproc1 "coproc2data">, <&coproc1
> "corpoc2ctrl">;

This isn't actually parseable. You need a known length of cells after a phandle.

Rob

Re: Generic DT binding for IPIs

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2341 bytes --]

On Wed, Dec 09, 2015 at 10:50:35AM -0600, Rob Herring wrote:
> On Wed, Dec 9, 2015 at 9:27 AM, Qais Yousef <qais.yousef-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:
> > Hi,
> >
> > On 10/22/2015 12:55 PM, Jason Cooper wrote:
> >>
> >> On Thu, Oct 22, 2015 at 11:44:16AM +0100, Qais Yousef wrote:
> >>>
> >>> Is there anything more I can do to get more attention about this? I
> >>> think Marc's suggestion is more generic and future proof, if I send
> >>> RFC patches for that would this be better?
> >>
> >> Please do.
> >
> >
> > Unfortunately I haven't had a chance to get around writing the patches yet.
> > I came up with a different description though that I thought maybe worth
> > sharing
> > to see if there's any opinion about it before the actual work being done.
> 
> I've not given this too much thought, but here's my initial thoughts.
> 
> >
> > To summarise, the problem I am trying to solve is that we have a type of
> > coprocessors which share the interrupt controller with Linux, hence the IPI
> > mechanism this controller uses. I've been working with Thomas on
> > implementing
> > a generic API to allocate IPIs for coprocesors and a way for drivers to send
> > these IPIs [1].
> >
> > To complement this new API, we need a mechanism to describe this in
> > device tree so a driver that wants to allocate an IPI can have this done
> > automatically for it like we handle interrupts.
> >
> > What I have in mind is:
> >
> >      coproc {
> >              ipi-parent = <&gic>;
> >
> >              ipis = <CPU_VALUE IPI_SPEC>;
> >              ipi-names = "in";
> >      };
> >
> > This will allocate an IPI to go to cpu @CPU_VALUE passing @IPI_SPEC as
> > parameters to the controller. Which means we need a new ipi-cells to
> > define how many cells are in ipis property. Note the new ipi-parent too.
> 
> These are still interrupts, so I'd prefer to use or extend the
> interrupt binding if possible.

I agree.  It should be possible to just describe these as interrupts,
with the interrupt-parent being a special interrupt controller node to
represent these IPIs.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Generic DT binding for IPIs

[Qais Yousef]

On 12/09/2015 04:50 PM, Rob Herring wrote:
> On Wed, Dec 9, 2015 at 9:27 AM, Qais Yousef <qais.yousef-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:
>
>> What I have in mind is:
>>
>>       coproc {
>>               ipi-parent = <&gic>;
>>
>>               ipis = <CPU_VALUE IPI_SPEC>;
>>               ipi-names = "in";
>>       };
>>
>> This will allocate an IPI to go to cpu @CPU_VALUE passing @IPI_SPEC as
>> parameters to the controller. Which means we need a new ipi-cells to
>> define how many cells are in ipis property. Note the new ipi-parent too.
> These are still interrupts, so I'd prefer to use or extend the
> interrupt binding if possible.

The IPIs have two properties that are different from a regular interrupts:

     1. An IPI is not only received, it could also be sent.
     2. The IPI is dynamic. There's an actual allocation from a pool of 
available
         IPIs happening when we ask for one to be reserved.

The difference might be borderline..

Do you have any rough idea on what a possible extension could look like? 
Reusing means writing less code, which is always better of course :)

By the way, on MIPS GIC, we can use interrupts property to describe an 
IPI the host system will receive. But to send one to the coprocessor, we 
need to define an outgoing IPI.

In this case, the firmware will be hardcoded to send an interrupt to a 
specific hwirq, so one can then describe it in DT as a regular interrupt 
to the host system. Hardcoding is not ideal and less portable though.

>> ipis property also is similar to interrupts, so using it would be easier
>> (I think).
>>
>> If we have 2 coprocessors that want to communicate using IPIs that are
>> managed by the host we use ipi-refs property to refer to IPIs defined in
>> another node.
>>
>>       coproc1 {
>>               ipis = <CPU1>, <CPU2>, <CPU2>;
> Don't you need to specify a certain IPI number in addition to which
> cpu is the target?

No. The way IPI reserving works is we just need to specify the target 
CPU(s).

>
> I'm thinking the cpu target could be part of the interrupts property
> flags field or something.

I'll look more at this option.

>
>>               ipi-names = "in", "coproc2data", "coproc2ctrl";
> -names should be optional in general. So define something that works
> without them.

If it's not specified, then the driver can get the definition by index 
and it would have to define the order it expects the IPIs in the binding?

>>       };
>>
>>       coproc2 {
>>               ipi-refs = <&coproc1 "in">, <&coproc1 "coproc2data">, <&coproc1
>> "corpoc2ctrl">;
> This isn't actually parseable. You need a known length of cells after a phandle.
>

To clarify, what you're saying we can't pass strings, right?

Thanks for your comments!

Thanks,
Qais
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Generic DT binding for IPIs

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4230 bytes --]

On Thu, Dec 10, 2015 at 10:20:49AM +0000, Qais Yousef wrote:
> On 12/09/2015 04:50 PM, Rob Herring wrote:
> >On Wed, Dec 9, 2015 at 9:27 AM, Qais Yousef <qais.yousef-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:
> >
> >>What I have in mind is:
> >>
> >>      coproc {
> >>              ipi-parent = <&gic>;
> >>
> >>              ipis = <CPU_VALUE IPI_SPEC>;
> >>              ipi-names = "in";
> >>      };
> >>
> >>This will allocate an IPI to go to cpu @CPU_VALUE passing @IPI_SPEC as
> >>parameters to the controller. Which means we need a new ipi-cells to
> >>define how many cells are in ipis property. Note the new ipi-parent too.
> >These are still interrupts, so I'd prefer to use or extend the
> >interrupt binding if possible.
> 
> The IPIs have two properties that are different from a regular interrupts:
> 
>     1. An IPI is not only received, it could also be sent.

Any interrupt is sent by the device, received by an interrupt
controller, so this isn't really anything fundamentally different.

>     2. The IPI is dynamic. There's an actual allocation from a pool of
> available
>         IPIs happening when we ask for one to be reserved.

It's not really clear to me what that means, and why it requires any
particular different information in the device tree.

> The difference might be borderline..
> 
> Do you have any rough idea on what a possible extension could look like?
> Reusing means writing less code, which is always better of course :)
> 
> By the way, on MIPS GIC, we can use interrupts property to describe an IPI
> the host system will receive. But to send one to the coprocessor, we need to
> define an outgoing IPI.

Ah, ok, so is what you're trying to describe here (from the host OS
and CPU point of view) a purely outgoing signal to the coproc?  

> In this case, the firmware will be hardcoded to send an interrupt to a
> specific hwirq, so one can then describe it in DT as a regular interrupt to
> the host system. Hardcoding is not ideal and less portable though.

Or is the signal that goes to the coproc then somehow being translated
into a host interrupt?  If that's so you should be able to represent
the coproc as an interrupt controller or interrupt nexus.




> >>ipis property also is similar to interrupts, so using it would be easier
> >>(I think).
> >>
> >>If we have 2 coprocessors that want to communicate using IPIs that are
> >>managed by the host we use ipi-refs property to refer to IPIs defined in
> >>another node.
> >>
> >>      coproc1 {
> >>              ipis = <CPU1>, <CPU2>, <CPU2>;
> >Don't you need to specify a certain IPI number in addition to which
> >cpu is the target?
> 
> No. The way IPI reserving works is we just need to specify the target
> CPU(s).
> 
> >
> >I'm thinking the cpu target could be part of the interrupts property
> >flags field or something.
> 
> I'll look more at this option.
> 
> >
> >>              ipi-names = "in", "coproc2data", "coproc2ctrl";
> >-names should be optional in general. So define something that works
> >without them.
> 
> If it's not specified, then the driver can get the definition by index and
> it would have to define the order it expects the IPIs in the binding?
> 
> >>      };
> >>
> >>      coproc2 {
> >>              ipi-refs = <&coproc1 "in">, <&coproc1 "coproc2data">, <&coproc1
> >>"corpoc2ctrl">;
> >This isn't actually parseable. You need a known length of cells after a phandle.
> >
> 
> To clarify, what you're saying we can't pass strings, right?

So, I'm not entirely sure what point Rob was making.  The above
certainly isn't valid dts syntax - strings can't appear within
the < > construct.  But if you make the obvious fix to:
    ipi-refs = <&coproc1>, "in", <&coproc1>, "coproc2data";

then it's certainly a parseable property format.  It's kind of clunky
mixing integers and strings that way, but it's possible and there are
existing bindings using properties in a similar format.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Generic DT binding for IPIs

[Qais Yousef]

On 12/11/2015 12:39 AM, David Gibson wrote:
> On Thu, Dec 10, 2015 at 10:20:49AM +0000, Qais Yousef wrote:
>>
>> The IPIs have two properties that are different from a regular interrupts:
>>
>>      1. An IPI is not only received, it could also be sent.
> Any interrupt is sent by the device, received by an interrupt
> controller, so this isn't really anything fundamentally different.

No they're not fundamentally different. It's just the way they're 
created and used.

>>      2. The IPI is dynamic. There's an actual allocation from a pool of
>> available
>>          IPIs happening when we ask for one to be reserved.
> It's not really clear to me what that means, and why it requires any
> particular different information in the device tree.

Maybe it would help to look at the new IPI reservation API?

     https://lkml.org/lkml/2015/12/8/249

>> The difference might be borderline..
>>
>> Do you have any rough idea on what a possible extension could look like?
>> Reusing means writing less code, which is always better of course :)
>>
>> By the way, on MIPS GIC, we can use interrupts property to describe an IPI
>> the host system will receive. But to send one to the coprocessor, we need to
>> define an outgoing IPI.
> Ah, ok, so is what you're trying to describe here (from the host OS
> and CPU point of view) a purely outgoing signal to the coproc?

Yes.

>> In this case, the firmware will be hardcoded to send an interrupt to a
>> specific hwirq, so one can then describe it in DT as a regular interrupt to
>> the host system. Hardcoding is not ideal and less portable though.
> Or is the signal that goes to the coproc then somehow being translated
> into a host interrupt?  If that's so you should be able to represent
> the coproc as an interrupt controller or interrupt nexus.
>

I'm not sure I understood you completely but no, there's no translation 
happening. When the IPI is allocated it would be routed
to the coproc. When the host wants to send a signal, it'll use the 
allocated hwirq value (indirectly via the virq) to write to a register, 
which will cause an interrupt to be generated at the coproc.


>>>>       };
>>>>
>>>>       coproc2 {
>>>>               ipi-refs = <&coproc1 "in">, <&coproc1 "coproc2data">, <&coproc1
>>>> "corpoc2ctrl">;
>>> This isn't actually parseable. You need a known length of cells after a phandle.
>>>
>> To clarify, what you're saying we can't pass strings, right?
> So, I'm not entirely sure what point Rob was making.  The above
> certainly isn't valid dts syntax - strings can't appear within
> the < > construct.  But if you make the obvious fix to:
>      ipi-refs = <&coproc1>, "in", <&coproc1>, "coproc2data";
>
> then it's certainly a parseable property format.  It's kind of clunky
> mixing integers and strings that way, but it's possible and there are
> existing bindings using properties in a similar format.
>

Ah OK thanks! I think this form would be handy to get the refs even if 
we end up reusing the interrupts property to allocate an IPI.

So if reusing the interrupts property is the right thing to do, do you 
(or anyone else) have a rough idea how this should look like?

Thanks,
Qais

Re: Generic DT binding for IPIs

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4608 bytes --]

On Fri, Dec 11, 2015 at 10:47:57AM +0000, Qais Yousef wrote:
> On 12/11/2015 12:39 AM, David Gibson wrote:
> >On Thu, Dec 10, 2015 at 10:20:49AM +0000, Qais Yousef wrote:
> >>
> >>The IPIs have two properties that are different from a regular interrupts:
> >>
> >>     1. An IPI is not only received, it could also be sent.
> >Any interrupt is sent by the device, received by an interrupt
> >controller, so this isn't really anything fundamentally different.
> 
> No they're not fundamentally different. It's just the way they're created
> and used.
> 
> >>     2. The IPI is dynamic. There's an actual allocation from a pool of
> >>available
> >>         IPIs happening when we ask for one to be reserved.
> >It's not really clear to me what that means, and why it requires any
> >particular different information in the device tree.
> 
> Maybe it would help to look at the new IPI reservation API?
> 
>     https://lkml.org/lkml/2015/12/8/249

Hmm.. not as much as I might have hoped.

From the API, it looks like you're reserving IPIs to signal between
different CPUs all of which are in Linux.  From your description here
it sounds like the coproc is a separate specialized thing not running
Linux (or at least, not the same Linux instance as the host OS which
this device tree is for).

> >>The difference might be borderline..
> >>
> >>Do you have any rough idea on what a possible extension could look like?
> >>Reusing means writing less code, which is always better of course :)
> >>
> >>By the way, on MIPS GIC, we can use interrupts property to describe an IPI
> >>the host system will receive. But to send one to the coprocessor, we need to
> >>define an outgoing IPI.
> >Ah, ok, so is what you're trying to describe here (from the host OS
> >and CPU point of view) a purely outgoing signal to the coproc?
> 
> Yes.

Ok.  In that case 'interrupts' is definitely *not* the right way to
describe this.  'interrupts' describes a signal going _from_ the node
in which it appears, _to_ the (host) cpu.  Or at least to an interrupt
controller which will generally forward it to the host cpu one way or
another.


> >>In this case, the firmware will be hardcoded to send an interrupt to a
> >>specific hwirq, so one can then describe it in DT as a regular interrupt to
> >>the host system. Hardcoding is not ideal and less portable though.
> >Or is the signal that goes to the coproc then somehow being translated
> >into a host interrupt?  If that's so you should be able to represent
> >the coproc as an interrupt controller or interrupt nexus.
> >
> 
> I'm not sure I understood you completely but no, there's no translation
> happening. When the IPI is allocated it would be routed
> to the coproc. When the host wants to send a signal, it'll use the allocated
> hwirq value (indirectly via the virq) to write to a register, which will
> cause an interrupt to be generated at the coproc.

Where is this magic register located?  In the host cpu?  In the
coproc?  In some special IPI controller?

> >>>>      };
> >>>>
> >>>>      coproc2 {
> >>>>              ipi-refs = <&coproc1 "in">, <&coproc1 "coproc2data">, <&coproc1
> >>>>"corpoc2ctrl">;
> >>>This isn't actually parseable. You need a known length of cells after a phandle.
> >>>
> >>To clarify, what you're saying we can't pass strings, right?
> >So, I'm not entirely sure what point Rob was making.  The above
> >certainly isn't valid dts syntax - strings can't appear within
> >the < > construct.  But if you make the obvious fix to:
> >     ipi-refs = <&coproc1>, "in", <&coproc1>, "coproc2data";
> >
> >then it's certainly a parseable property format.  It's kind of clunky
> >mixing integers and strings that way, but it's possible and there are
> >existing bindings using properties in a similar format.
> >
> 
> Ah OK thanks! I think this form would be handy to get the refs even if we
> end up reusing the interrupts property to allocate an IPI.
> 
> So if reusing the interrupts property is the right thing to do, do you (or
> anyone else) have a rough idea how this should look like?

So as noted above, I'm now sure that 'interrupts' is not the right
thing.  I'm trying to understand the coprocs and the ipi mechanism a
bit better to work out if there is something existing which would make
sense, or if we do need something entirely new.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Generic DT binding for IPIs

[Qais Yousef]

[-- Attachment #1: Type: text/plain, Size: 1547 bytes --]

> > Maybe it would help to look at the new IPI reservation API?
> >
> >     https://lkml.org/lkml/2015/12/8/249
>
> Hmm.. not as much as I might have hoped.
>
> From the API, it looks like you're reserving IPIs to signal between
> different CPUs all of which are in Linux.  From your description here
> it sounds like the coproc is a separate specialized thing not running
> Linux (or at least, not the same Linux instance as the host OS which
> this device tree is for).

No the CPUs aren't only Linux CPUs. It should be any CPU that the interrupt
controller can talk to.
That CPU can be one of Linux CPUs, or a coproc.

> > I'm not sure I understood you completely but no, there's no translation
> > happening. When the IPI is allocated it would be routed
> > to the coproc. When the host wants to send a signal, it'll use the
allocated
> > hwirq value (indirectly via the virq) to write to a register, which will
> > cause an interrupt to be generated at the coproc.
>
> Where is this magic register located?  In the host cpu?  In the
> coproc?  In some special IPI controller?

The IPI controller. In my use case, the IPI controller is the same as the
interrupt controller used by Linux. Generally they don't have to be the
same though.

> So as noted above, I'm now sure that 'interrupts' is not the right
> thing.  I'm trying to understand the coprocs and the ipi mechanism a
> bit better to work out if there is something existing which would make
> sense, or if we do need something entirely new.

Let me know if I can help.

Thanks,
Qais

[-- Attachment #2: Type: text/html, Size: 2008 bytes --]

Re: Generic DT binding for IPIs

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3364 bytes --]

On Thu, Dec 17, 2015 at 11:31:14AM +0000, Qais Yousef wrote:
> > > Maybe it would help to look at the new IPI reservation API?
> > >
> > >     https://lkml.org/lkml/2015/12/8/249
> >
> > Hmm.. not as much as I might have hoped.
> >
> > From the API, it looks like you're reserving IPIs to signal between
> > different CPUs all of which are in Linux.  From your description here
> > it sounds like the coproc is a separate specialized thing not running
> > Linux (or at least, not the same Linux instance as the host OS which
> > this device tree is for).
> 
> No the CPUs aren't only Linux CPUs. It should be any CPU that the interrupt
> controller can talk to.
> That CPU can be one of Linux CPUs, or a coproc.

Ok.. but it looks like anything you can reserve an IPI for must be
cpu_possible_mask, so I'm guessing they're CPUs that could be CPUs,
even if they're not at the present time.  Is that right?

I guess the point is that the targets of these IPIs are participating
in the same coherency fabric as the host CPUs, yes?

> > > I'm not sure I understood you completely but no, there's no translation
> > > happening. When the IPI is allocated it would be routed
> > > to the coproc. When the host wants to send a signal, it'll use the
> allocated
> > > hwirq value (indirectly via the virq) to write to a register, which will
> > > cause an interrupt to be generated at the coproc.
> >
> > Where is this magic register located?  In the host cpu?  In the
> > coproc?  In some special IPI controller?
> 
> The IPI controller. In my use case, the IPI controller is the same as the
> interrupt controller used by Linux. Generally they don't have to be the
> same though.

Ok.  So, the one existing thing I can think of which might fit -
although it seems a bit odd - is the gpio binding.  I'm not that
familiar with the binding, so it's possible I've overlooked something
that would make it unsuitable, still..

I think you might be able to treat the IPIs bound for a coproc as
gpios to that coproc device, with the IPI controller designated as the
gpio controller.  The driver for this gpio controller would know that
it uses the IPI reservation mechanism to trigger those "gpio" lines.

This scheme would have the advatage that if you ever have a version of
the coproc that exists as a separate piece of hardware *not* belonging
to the same coherency fabric as the host cpu - and with it's inbound
interrupts instead triggered by "real" gpios - then that should, at
least in theory, be supported without host kernel changes.

> > So as noted above, I'm now sure that 'interrupts' is not the right
> > thing.  I'm trying to understand the coprocs and the ipi mechanism a
> > bit better to work out if there is something existing which would make
> > sense, or if we do need something entirely new.
> 
> Let me know if I can help.

I don't know what sort of software is running on the coproc.  If it
also uses a device tree, then the device tree from the coproc point of
view *would* represent these inbound IPIs as 'interrupts' properties -
probably on some node representing the host cpu / system.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Generic DT binding for IPIs

[Rob Herring]

On Wed, Oct 14, 2015 at 5:18 AM, Qais Yousef <qais.yousef-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:
> Hi,
>
> This is an attempt to revive a discussion on the right list this time with
> all the correct people hopefully on CC.

devicetree-spec would be more appropriate list for something like this.

> While trying to upstream a driver, Thomas and Marc Zyngier pointed out the
> need for a generic IPI support in the kernel to allow driver to reserve and
> send ones. Hopefully my latest RFC patch will help to clarify what's being
> done.
>
>     https://lkml.org/lkml/2015/10/13/227
>
> We need a generic DT binding support to accompany that to allow a driver to
> reserve an IPI using this new mechanism.
>
> MarcZ had the following suggestion:
>
>     https://lkml.org/lkml/2015/8/24/628
>
> Which in summary is
>
>         mydevice@f0000000 {
>                 interrupt-source = <&intc INT_SPEC 2 &inttarg1 &inttarg1>;

What is INT_SPEC and "2"? A drawing of the h/w connections and then
what the binding looks like would be helpful.

>         };
>
>         inttarg1: mydevice@f1000000 {
>                 interrupt-sink = <&intc HWAFFINITY1>;

What is HWAFFINITY1? I want to be able to see if say this value is 1,
then the affinity is for cpu0.

>         };
>
>         inttarg2: cpu@1 {
>                 interrupt-sink = <&intc HWAFFINITY2>;
>         };
>
>
> interrupt-sink requests to reserve an IPI that it will receive at HWAFFINITY
> cpumask. interrupt-source will not do any reservation. It will simply
> connect an IPI reserved by interrupt-sink to the device that will be
> responsible for generating that IPI. This description should allow
> connecting any 2 devices.
> Correct me Marc if I got it wrong please.
>
> I suggested a simplification by assuming that IPIs will only be between host
> OS and a coprocessor which would gives us this form which I think is easier
> to deal with
>
>         coprocessor {
>              interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>;
>              interrupt-sink = <&intc INT_SPEC CPU_HWAFFINITY>;
>         }
>
>
> interrupt-source here reserves an IPI to be sent from host OS to coprocessor
> at COP_HWAFFINITY. interrupt-sink will reserve an IPI to be received by host
> OS at CPU_HWAFFINITY. Less generic but I don't know how important it is for
> host OS to setup IPIs between 2 external coprocessors and whether it should
> really be doing that.

Could we use the existing interrupts binding for interrupt-sink?

>
> What do the DT experts think? Any preference or a better suggestion?

Depends how you would assign coproc to coproc IPIs in a system. It may
be fixed in firmware, or more complex coprocs may read the dtb.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Generic DT binding for IPIs

[Qais Yousef]

On 10/22/2015 02:43 PM, Rob Herring wrote:
> On Wed, Oct 14, 2015 at 5:18 AM, Qais Yousef <qais.yousef@imgtec.com> wrote:
>> Hi,
>>
>> This is an attempt to revive a discussion on the right list this time with
>> all the correct people hopefully on CC.
> devicetree-spec would be more appropriate list for something like this.

Thanks I didn't know about it.

>
>> While trying to upstream a driver, Thomas and Marc Zyngier pointed out the
>> need for a generic IPI support in the kernel to allow driver to reserve and
>> send ones. Hopefully my latest RFC patch will help to clarify what's being
>> done.
>>
>>      https://lkml.org/lkml/2015/10/13/227
>>
>> We need a generic DT binding support to accompany that to allow a driver to
>> reserve an IPI using this new mechanism.
>>
>> MarcZ had the following suggestion:
>>
>>      https://lkml.org/lkml/2015/8/24/628
>>
>> Which in summary is
>>
>>          mydevice@f0000000 {
>>                  interrupt-source = <&intc INT_SPEC 2 &inttarg1 &inttarg1>;
> What is INT_SPEC and "2"? A drawing of the h/w connections and then
> what the binding looks like would be helpful.

INT_SPEC are the interrupt controller specific parameters. I think 2 
refers to how many 'interrupt-sink' references it's being passed.

>
>>          };
>>
>>          inttarg1: mydevice@f1000000 {
>>                  interrupt-sink = <&intc HWAFFINITY1>;
> What is HWAFFINITY1? I want to be able to see if say this value is 1,
> then the affinity is for cpu0.

HWAFFINITY is the CPU to map the IPI to. The actual value would be 
specific to the interrupt controller. The controller implementation can 
always provide a set of defines if the mapping is not straightforward.

>
>>          };
>>
>>          inttarg2: cpu@1 {
>>                  interrupt-sink = <&intc HWAFFINITY2>;
>>          };
>>
>>
>> interrupt-sink requests to reserve an IPI that it will receive at HWAFFINITY
>> cpumask. interrupt-source will not do any reservation. It will simply
>> connect an IPI reserved by interrupt-sink to the device that will be
>> responsible for generating that IPI. This description should allow
>> connecting any 2 devices.
>> Correct me Marc if I got it wrong please.
>>
>> I suggested a simplification by assuming that IPIs will only be between host
>> OS and a coprocessor which would gives us this form which I think is easier
>> to deal with
>>
>>          coprocessor {
>>               interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>;
>>               interrupt-sink = <&intc INT_SPEC CPU_HWAFFINITY>;
>>          }
>>
>>
>> interrupt-source here reserves an IPI to be sent from host OS to coprocessor
>> at COP_HWAFFINITY. interrupt-sink will reserve an IPI to be received by host
>> OS at CPU_HWAFFINITY. Less generic but I don't know how important it is for
>> host OS to setup IPIs between 2 external coprocessors and whether it should
>> really be doing that.
> Could we use the existing interrupts binding for interrupt-sink?

No. interrupt-sink will cause an IPI to be dynamically allocated and 
mapped to that processor. Of course, if the connection is hardwired then 
interrupts property is the right thing to use.

>
>> What do the DT experts think? Any preference or a better suggestion?
> Depends how you would assign coproc to coproc IPIs in a system. It may
> be fixed in firmware, or more complex coprocs may read the dtb.

OK. I'll try to cook some RFC patches that implement this which 
hopefully will make it easier to review and build on.

Thanks,
Qais