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
On Wed, Dec 9, 2015 at 9:27 AM, Qais Yousef <[email protected]> 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
On Wed, Dec 09, 2015 at 10:50:35AM -0600, Rob Herring wrote:
> On Wed, Dec 9, 2015 at 9:27 AM, Qais Yousef <[email protected]> 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
On 12/09/2015 04:50 PM, Rob Herring wrote:
> On Wed, Dec 9, 2015 at 9:27 AM, Qais Yousef <[email protected]> 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
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 <[email protected]> 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
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
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