Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Nathan Whitehorn]

That's a good idea, Adrian. I've made a wiki page at 
https://wiki.freebsd.org/Complicated_Interrupts describing this and 
moved the discussion to freebsd-arch@

-Nathan

On 07/24/16 11:48, Adrian Chadd wrote:

[snip]

Hi,

I think a lot of this whole discussion stems from a reasonably unclear
shared idea of where intrng is supposed to be going and what it's
supposed to be doing. I'm sure everyone has their own ideas of what it
is for their own needs, but this thread shows a clear separation
between the original writers and people working on supporting their
own platforms.

So, my suggestion - also so I can understand it, since I kinda need it
now for the atheros mips ports! - is maybe something gets written up
in the wiki that's a more detailed technical overview of intrng,
including the problems its trying to solve. Then, instead of flinging
things around on the commit mailing list, people edit the wiki and
discuss on -arm about the specific details.

I think that'll avoid a lot of what's going on.

(And honestly, I'd like it more sorted out sooner rather than later,
as I really want to start attacking the QCA ARM ports (IPQ4019) if
it's not already done, and that's all interrupt/gpio/etc gymnastics we
need INTRNG for.)

So, Nathan, wanna start a wiki page with the /current/ implementation
that's in -HEAD and what the intention was from the PPC side?



-adrian



___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Adrian Chadd]

[snip]

Hi,

I think a lot of this whole discussion stems from a reasonably unclear
shared idea of where intrng is supposed to be going and what it's
supposed to be doing. I'm sure everyone has their own ideas of what it
is for their own needs, but this thread shows a clear separation
between the original writers and people working on supporting their
own platforms.

So, my suggestion - also so I can understand it, since I kinda need it
now for the atheros mips ports! - is maybe something gets written up
in the wiki that's a more detailed technical overview of intrng,
including the problems its trying to solve. Then, instead of flinging
things around on the commit mailing list, people edit the wiki and
discuss on -arm about the specific details.

I think that'll avoid a lot of what's going on.

(And honestly, I'd like it more sorted out sooner rather than later,
as I really want to start attacking the QCA ARM ports (IPQ4019) if
it's not already done, and that's all interrupt/gpio/etc gymnastics we
need INTRNG for.)

So, Nathan, wanna start a wiki page with the /current/ implementation
that's in -HEAD and what the intention was from the PPC side?



-adrian
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Michal Meloun]

I'm sorry for the blank response sent by mistake.

Dne 24.07.2016 v 17:32 Nathan Whitehorn napsal(a):
>
>
> On 07/24/16 00:45, Michal Meloun wrote:
>> Dne 23.07.2016 v 20:35 Nathan Whitehorn napsal(a):
>>>
>>>
>>> On 07/23/16 03:45, Michal Meloun wrote:
 Dne 21.07.2016 v 17:53 Nathan Whitehorn napsal(a):
>
> On PowerPC, GENERIC64 supports FDT systems (some IBM hardware),
> OFW systems (Macs, some IBM hardware), systems with no device
> trees at all (old-style PS3), and systems with a mixture of device
> tree and no device tree (new-style PS3). On these, there is a
> mixture of "real" interrupts and GPIO-type interrupts. There is no
> limitation that this be used only for device-tree-type systems.
>
> The system requires two things: an interrupt domain key and an
> arbitrary unique byte string describing the interrupt. When
> running with a device tree, these are set to the phandle of the
> interrupt-parent and the contents of the device tree interrupt
> specifier, respectively, and the system was of course developed
> with that in mind. But they don't need to be, and often aren't.
> You could make the domain an element of an enum (where "ACPI" is a
> choice, for instance -- this is what PS3 does), or set it to a
> pointer to a device_t, or really anything you like. Similarly, the
> interrupt specifier is totally free-form.

 Yes, I agree. and i think that we followed the same direction. But
 i see two problems with this approach.
 - in some cases (OFW, device_t)  you don't have  control over
 domain key value, so you cannot guarantee its uniqueness.
   Of course, probability of collision is low, but it is.
>>>
>>> We could solve this in a number of ways, for example widening to 64
>>> bits, or adding another value (domain type, for example). You could
>>> also make an acpi_bus_map_intr() to go with the OFW one that connect
>>> in some machine-dependent code if you have fundamentally
>>> incompatible bus enumeration mechanisms that you expect to exist
>>> simultaneously -- but, of course, no systems like this seem to
>>> actually exist, so the problem is both easily solved and totally
>>> theoretical.
>>>
 - within ofw_bus_map_intr() (or later - at the time when byte
 string must be decoded)  you are not able (easily) to differentiate
   between different formats, thus you are not able to select
 appropriate  decoder. The GPIO controller, for example,
   must be able to handle interrupts defined by standard OFW
 property, or by  pair concurrently.
>>>
>>> In principle, you could solve that as above, or by registering a
>>> second interrupt domain for the same controller.
>>>
>>> In practice, it doesn't matter since, in the GPIO case, for example,
>>> the GPIO controller is never itself also a normal interrupt
>>> controller (i.e. the GIC and GPIO controller are always different
>>> devices). As such, the theoretical does not occur in practice.
>> form
>> https://svnweb.freebsd.org/base/head/sys/gnu/dts/arm/tegra124-jetson-tk1.dts?revision=295436=markup#l1380
>> "interrupts = ; "
>> Do you want more examples ?
>
> Those have the identical format to the GPIO properties, because they
> are the same thing. So it works out of the box. Do you have examples
> of something that *doesn't work*?
>>
>>>
 For this reason we makes domain key composite, in our model, the
 domain key consist of "domain key type"
 and "domain key value". This composite key guarantees uniqueness
 and  it also allows to select proper parser for byte string.
>>>
>>> Yes, but this solves what is a nonexistant problem by making the
>>> system substantially less flexible and more invasive. Which is not a
>>> good tradeoff.
>>>
>> I think that existence of problem is confirmed in the above example .
>> Quote from previous paragraph:
>> "We could solve this in a number of ways, ... , or adding another
>> value (domain type, for example)."
>> What can I say more ...
>
> Except that the example you gave *is not an example* of the problem
> you are describing. You would only end up with a problem if:
> 1) You had interrupts in a GPIO property rather than in an interrupts
> property (or equivalent).
> 2) *And* you had interrupts on GPIOs in an interrupts property (or
> equivalent)
> 3) *and* those are encoded differently
> 4) *and* the different encodings use the same number of cells
> 5) *and* are not otherwise distinguishable
>
> Does that ever actually happen, in the real world, on any device tree?
> You could imagine any kind of messed up thing you want, but we
> shouldn't structure APIs around them, especially given that the
> current alternative you are proposing has real, concrete problems on
> real hardware that actually exists.
>
Allow me to respond to this issue only.  I think that this main issue,
everything else looks resolvable for me 

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Michal Meloun]

Dne 24.07.2016 v 17:32 Nathan Whitehorn napsal(a):
>
>
> On 07/24/16 00:45, Michal Meloun wrote:
>> Dne 23.07.2016 v 20:35 Nathan Whitehorn napsal(a):
>>>
>>>
>>> On 07/23/16 03:45, Michal Meloun wrote:
 Dne 21.07.2016 v 17:53 Nathan Whitehorn napsal(a):
>
> On PowerPC, GENERIC64 supports FDT systems (some IBM hardware),
> OFW systems (Macs, some IBM hardware), systems with no device
> trees at all (old-style PS3), and systems with a mixture of device
> tree and no device tree (new-style PS3). On these, there is a
> mixture of "real" interrupts and GPIO-type interrupts. There is no
> limitation that this be used only for device-tree-type systems.
>
> The system requires two things: an interrupt domain key and an
> arbitrary unique byte string describing the interrupt. When
> running with a device tree, these are set to the phandle of the
> interrupt-parent and the contents of the device tree interrupt
> specifier, respectively, and the system was of course developed
> with that in mind. But they don't need to be, and often aren't.
> You could make the domain an element of an enum (where "ACPI" is a
> choice, for instance -- this is what PS3 does), or set it to a
> pointer to a device_t, or really anything you like. Similarly, the
> interrupt specifier is totally free-form.

 Yes, I agree. and i think that we followed the same direction. But
 i see two problems with this approach.
 - in some cases (OFW, device_t)  you don't have  control over
 domain key value, so you cannot guarantee its uniqueness.
   Of course, probability of collision is low, but it is.
>>>
>>> We could solve this in a number of ways, for example widening to 64
>>> bits, or adding another value (domain type, for example). You could
>>> also make an acpi_bus_map_intr() to go with the OFW one that connect
>>> in some machine-dependent code if you have fundamentally
>>> incompatible bus enumeration mechanisms that you expect to exist
>>> simultaneously -- but, of course, no systems like this seem to
>>> actually exist, so the problem is both easily solved and totally
>>> theoretical.
>>>
 - within ofw_bus_map_intr() (or later - at the time when byte
 string must be decoded)  you are not able (easily) to differentiate
   between different formats, thus you are not able to select
 appropriate  decoder. The GPIO controller, for example,
   must be able to handle interrupts defined by standard OFW
 property, or by  pair concurrently.
>>>
>>> In principle, you could solve that as above, or by registering a
>>> second interrupt domain for the same controller.
>>>
>>> In practice, it doesn't matter since, in the GPIO case, for example,
>>> the GPIO controller is never itself also a normal interrupt
>>> controller (i.e. the GIC and GPIO controller are always different
>>> devices). As such, the theoretical does not occur in practice.
>> form
>> https://svnweb.freebsd.org/base/head/sys/gnu/dts/arm/tegra124-jetson-tk1.dts?revision=295436=markup#l1380
>> "interrupts = ; "
>> Do you want more examples ?
>
> Those have the identical format to the GPIO properties, because they
> are the same thing. So it works out of the box. Do you have examples
> of something that *doesn't work*?
>
>>
>>>
 For this reason we makes domain key composite, in our model, the
 domain key consist of "domain key type"
 and "domain key value". This composite key guarantees uniqueness
 and  it also allows to select proper parser for byte string.
>>>
>>> Yes, but this solves what is a nonexistant problem by making the
>>> system substantially less flexible and more invasive. Which is not a
>>> good tradeoff.
>>>
>> I think that existence of problem is confirmed in the above example .
>> Quote from previous paragraph:
>> "We could solve this in a number of ways, ... , or adding another
>> value (domain type, for example)."
>> What can I say more ...
>
> Except that the example you gave *is not an example* of the problem
> you are describing. You would only end up with a problem if:
> 1) You had interrupts in a GPIO property rather than in an interrupts
> property (or equivalent).
> 2) *And* you had interrupts on GPIOs in an interrupts property (or
> equivalent)
> 3) *and* those are encoded differently
> 4) *and* the different encodings use the same number of cells
> 5) *and* are not otherwise distinguishable
>
> Does that ever actually happen, in the real world, on any device tree?
> You could imagine any kind of messed up thing you want, but we
> shouldn't structure APIs around them, especially given that the
> current alternative you are proposing has real, concrete problems on
> real hardware that actually exists.
>
>>
>>
>
> [snip]
>
  
>
>>
>>>
> It is much easier to do this correctly at bus attach time when
> the resource lists are made 

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Nathan Whitehorn]

On 07/24/16 08:45, Warner Losh wrote:

On Sun, Jul 24, 2016 at 9:32 AM, Nathan Whitehorn
 wrote:

This will make this much harder to untangle, unfortunately, and probably
means we are stuck with this as a rump API in stable/11.

The time to have had this discussion was 9 months ago when it first started
to appear in the tree and on differential and on the mailing lists.

I'm also not convinced that 'planes' would be the right way to solve the
interrupt issues. There's been talk about it for a long time, but no action.
The relationships in FDT are DAGs, not trees, and newbus is inherently
tree-based.

Warner



I at least had never seen this code until it appeared in the tree and 
went through a phabricator review during code slush in which no one 
approved it.


The general discussion of INTRNG 9 months ago was great, and I wrote 
some of the code. It is a big step forward. This particular code, 
however, is not consistent with the understanding of what INTRNG would 
be that I got from that discussion. The actual discussion on mailing 
lists seems to consist only of this cryptic email on an ARM list 
(https://lists.freebsd.org/pipermail/freebsd-arm/2016-June/013976.html) 
with no meaningful content right before the commit and some phabricator 
reviews that no one signed off on, that do not include all the relevant 
maintainers, and that end in one case with open questions followed by a 
commit. Since this fundamentally affects parts of kern/ and newbus, this 
needed to be on freebsd-arch for a month at the *very* least and to have 
positive approval.


Given how this was handled and where we are in the release cycle, I 
don't know what the right course of action is; I see no good outcomes at 
this point.


The core problem is that the code in this commit series duplicates an 
existing architecture that solves all the problems it purports, but 
can't ever be portable to other platforms because of its dependency 
model. As such, it will either (a) bitrot or (b) sit in the tree as a 
permanent, inflexible, ARM-only adjunct to the systems used on other 
platforms, filling kern and dev/ofw with progressively more #ifdef. We 
worked very hard to *remove* an API very similar from this on MIPS in 
the initial parts of INTRNG because it crippled the flexibility of the 
system. Having it appear again, under the radar, during code slush with 
no meaningful review and the author unreachable right before a major 
release is extremely disappointing.

-Nathan
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Warner Losh]

On Sun, Jul 24, 2016 at 9:32 AM, Nathan Whitehorn
 wrote:
> This will make this much harder to untangle, unfortunately, and probably
> means we are stuck with this as a rump API in stable/11.

The time to have had this discussion was 9 months ago when it first started
to appear in the tree and on differential and on the mailing lists.

I'm also not convinced that 'planes' would be the right way to solve the
interrupt issues. There's been talk about it for a long time, but no action.
The relationships in FDT are DAGs, not trees, and newbus is inherently
tree-based.

Warner
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Nathan Whitehorn]

On 07/24/16 00:45, Michal Meloun wrote:

Dne 23.07.2016 v 20:35 Nathan Whitehorn napsal(a):



On 07/23/16 03:45, Michal Meloun wrote:

Dne 21.07.2016 v 17:53 Nathan Whitehorn napsal(a):


On PowerPC, GENERIC64 supports FDT systems (some IBM hardware), OFW 
systems (Macs, some IBM hardware), systems with no device trees at 
all (old-style PS3), and systems with a mixture of device tree and 
no device tree (new-style PS3). On these, there is a mixture of 
"real" interrupts and GPIO-type interrupts. There is no limitation 
that this be used only for device-tree-type systems.


The system requires two things: an interrupt domain key and an 
arbitrary unique byte string describing the interrupt. When running 
with a device tree, these are set to the phandle of the 
interrupt-parent and the contents of the device tree interrupt 
specifier, respectively, and the system was of course developed 
with that in mind. But they don't need to be, and often aren't. You 
could make the domain an element of an enum (where "ACPI" is a 
choice, for instance -- this is what PS3 does), or set it to a 
pointer to a device_t, or really anything you like. Similarly, the 
interrupt specifier is totally free-form.


Yes, I agree. and i think that we followed the same direction. But i 
see two problems with this approach.
- in some cases (OFW, device_t)  you don't have  control over domain 
key value, so you cannot guarantee its uniqueness.

  Of course, probability of collision is low, but it is.


We could solve this in a number of ways, for example widening to 64 
bits, or adding another value (domain type, for example). You could 
also make an acpi_bus_map_intr() to go with the OFW one that connect 
in some machine-dependent code if you have fundamentally incompatible 
bus enumeration mechanisms that you expect to exist simultaneously -- 
but, of course, no systems like this seem to actually exist, so the 
problem is both easily solved and totally theoretical.


- within ofw_bus_map_intr() (or later - at the time when byte string 
must be decoded)  you are not able (easily) to differentiate
  between different formats, thus you are not able to select 
appropriate  decoder. The GPIO controller, for example,
  must be able to handle interrupts defined by standard OFW 
property, or by  pair concurrently.


In principle, you could solve that as above, or by registering a 
second interrupt domain for the same controller.


In practice, it doesn't matter since, in the GPIO case, for example, 
the GPIO controller is never itself also a normal interrupt 
controller (i.e. the GIC and GPIO controller are always different 
devices). As such, the theoretical does not occur in practice.
form 
https://svnweb.freebsd.org/base/head/sys/gnu/dts/arm/tegra124-jetson-tk1.dts?revision=295436=markup#l1380

"interrupts = ; "
Do you want more examples ?


Those have the identical format to the GPIO properties, because they are 
the same thing. So it works out of the box. Do you have examples of 
something that *doesn't work*?






For this reason we makes domain key composite, in our model, the 
domain key consist of "domain key type"
and "domain key value". This composite key guarantees uniqueness 
and  it also allows to select proper parser for byte string.


Yes, but this solves what is a nonexistant problem by making the 
system substantially less flexible and more invasive. Which is not a 
good tradeoff.



I think that existence of problem is confirmed in the above example .
Quote from previous paragraph:
"We could solve this in a number of ways, ... , or adding another 
value (domain type, for example)."

What can I say more ...


Except that the example you gave *is not an example* of the problem you 
are describing. You would only end up with a problem if:
1) You had interrupts in a GPIO property rather than in an interrupts 
property (or equivalent).
2) *And* you had interrupts on GPIOs in an interrupts property (or 
equivalent)

3) *and* those are encoded differently
4) *and* the different encodings use the same number of cells
5) *and* are not otherwise distinguishable

Does that ever actually happen, in the real world, on any device tree? 
You could imagine any kind of messed up thing you want, but we shouldn't 
structure APIs around them, especially given that the current 
alternative you are proposing has real, concrete problems on real 
hardware that actually exists.







[snip]









It is much easier to do this correctly at bus attach time when 
the resource lists are made (how PPC does it).


I don't agree. I don't agree. Making this at bus attach time 
leads into complicated 'virtual' IRQ infrastructure, with many 
unresolved corner cases.


Which unresolved corner cases? This has been working correctly on 
a number of platforms in both FreeBSD and Linux for many years.
Nope, it doesn't work for ARM yet (for GPIO interrupts for 
example)  and Linux uses 

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Michal Meloun]

Dne 23.07.2016 v 20:35 Nathan Whitehorn napsal(a):
>
>
> On 07/23/16 03:45, Michal Meloun wrote:
>> Dne 21.07.2016 v 17:53 Nathan Whitehorn napsal(a):
>>>
>>> On PowerPC, GENERIC64 supports FDT systems (some IBM hardware), OFW
>>> systems (Macs, some IBM hardware), systems with no device trees at
>>> all (old-style PS3), and systems with a mixture of device tree and
>>> no device tree (new-style PS3). On these, there is a mixture of
>>> "real" interrupts and GPIO-type interrupts. There is no limitation
>>> that this be used only for device-tree-type systems.
>>>
>>> The system requires two things: an interrupt domain key and an
>>> arbitrary unique byte string describing the interrupt. When running
>>> with a device tree, these are set to the phandle of the
>>> interrupt-parent and the contents of the device tree interrupt
>>> specifier, respectively, and the system was of course developed with
>>> that in mind. But they don't need to be, and often aren't. You could
>>> make the domain an element of an enum (where "ACPI" is a choice, for
>>> instance -- this is what PS3 does), or set it to a pointer to a
>>> device_t, or really anything you like. Similarly, the interrupt
>>> specifier is totally free-form.
>>
>> Yes, I agree. and i think that we followed the same direction. But i
>> see two problems with this approach.
>> - in some cases (OFW, device_t)  you don't have  control over domain
>> key value, so you cannot guarantee its uniqueness.
>>   Of course, probability of collision is low, but it is.
>
> We could solve this in a number of ways, for example widening to 64
> bits, or adding another value (domain type, for example). You could
> also make an acpi_bus_map_intr() to go with the OFW one that connect
> in some machine-dependent code if you have fundamentally incompatible
> bus enumeration mechanisms that you expect to exist simultaneously --
> but, of course, no systems like this seem to actually exist, so the
> problem is both easily solved and totally theoretical.
>
>> - within ofw_bus_map_intr() (or later - at the time when byte string
>> must be decoded)  you are not able (easily) to differentiate
>>   between different formats, thus you are not able to select
>> appropriate  decoder. The GPIO controller, for example,
>>   must be able to handle interrupts defined by standard OFW property,
>> or by  pair concurrently.
>
> In principle, you could solve that as above, or by registering a
> second interrupt domain for the same controller.
>
> In practice, it doesn't matter since, in the GPIO case, for example,
> the GPIO controller is never itself also a normal interrupt controller
> (i.e. the GIC and GPIO controller are always different devices). As
> such, the theoretical does not occur in practice.
form
https://svnweb.freebsd.org/base/head/sys/gnu/dts/arm/tegra124-jetson-tk1.dts?revision=295436=markup#l1380
"interrupts = ; "
Do you want more examples ?


>
>> For this reason we makes domain key composite, in our model, the
>> domain key consist of "domain key type"
>> and "domain key value". This composite key guarantees uniqueness and 
>> it also allows to select proper parser for byte string.
>
> Yes, but this solves what is a nonexistant problem by making the
> system substantially less flexible and more invasive. Which is not a
> good tradeoff.
>
I think that existence of problem is confirmed in the above example .
Quote from previous paragraph:
"We could solve this in a number of ways, ... , or adding another value
(domain type, for example)."
What can I say more ...

>> This is, imho, only one difference between us.
>
> One of many, yes.
>
>>
>>> You could, for instance, set it to one of the structures introduced
>>> in r301453 if you wanted to.
>>>
>>> I would have zero problems with changing the prototype of the
>>> existing dev/ofw function to something more generic in name, like:
>>>
>>> bus_map_intr(device_t dev, uintptr_t iparent, size_t intrlen, void
>>> *intr)
>>>
>>> instead of the existing equivalent:
>>>
>>> ofw_bus_map_intr(device_t dev, phandle_t iparent, int icells,
>>> pcell_t *intr)
>>>
>> Our bus_map_intr() method is not indeed as replacement of 
>> ofw_bus_map_intr(). Its evolution of "how we can store more complex
>> data to resource list (from bus enumerator) and transfer it  to
>> bus_allocate_resource() and/or to bus_setup_intr()" in driver
>> independent way. We found no reasonable way to do it, so we postponed
>> reading of properties to bus_allocate_resource() time.
>
> Right, but that is (a) a solved problem with ofw_bus_map_intr() and
> (b) this code doesn't solve it as completely. What does it let you do
> that the existing code does not? There has not been a single concrete
> example of something anyone wants to do on actual hardware so far in
> this discussion.
>
>> But now  jhb@ gives us alternative and I must say that this looks
>> like a clean and elegant way how to make this (assuming 

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Nathan Whitehorn]

On 07/23/16 04:04, Michal Meloun wrote:

Dne 21.07.2016 v 23:35 John Baldwin napsal(a):

On Thursday, July 21, 2016 01:37:42 PM Andrew Turner wrote:

On Wed, 20 Jul 2016 13:28:53 +0200
Michal Meloun  wrote:

Dne 19.07.2016 v 17:06 Nathan Whitehorn napsal(a):

2. It partially duplicates the functionality of OFW_BUS_MAP_INTR(),
but is both problematically more general and less flexible (it has
requirements on timing of PIC attachment vs. driver resource
allocation)

OFW_BUS_MAP_INTR()  can parse only OFW  based data and expect that
parsed data are magicaly stored within the call.
The new method, bus_map_intr(),  can parse data from multiple sources
(OFW, UEFI / ACPI, synthetic[gpio device + pin number]).  It also
returns parsed data back to caller.
And no, it  doesn't  add any additional timing requirements .

I've been looking at ACPI on arm64. So far I have not found the need
for this with ACPI as we don't need to send the data to the interrupt
controller driver to be parsed in the way OFW/FDT needs to.

ACPI though has a gross hack where we call BUS_CONFIG_INTR on the IRQ
in bus_alloc_resource().  What I had advocated in the discussions
leading up to this was to have some sort of opaque structure containing
a set of properties (the sort of thing bus_map_resource and make_dev_s
use) that was passed up at bus_setup_intr() time.  I think it should now
be passed up at bus_alloc_resource() time instead, but it would allow bus
drivers to "decorate" a SYS_RES_IRQ request as it goes up the device tree
with properties that the interrupt controller can then associate with
the IRQ cookie it allocates in its own code.  I would let the particular
structure have different layouts for different resource types.  On x86 we
would replace bus_config_intr by passing the level and trigger-mode in
this structure.  However, I could also see allowing the memattr to be
set for a SYS_RES_MEMORY resource so you could have a much shorter way
than an explicit bus_map_resource to map an entire BAR as WC for example:

 struct alloc_resource_args {
 size_t len;
 union {
 struct {
 enum intr_trigger trigger;
 enum intr_polarity polarity;
 } irq;
 struct {
 vm_memattr_t memattr;
 } memory;
 }
 }

...

 union alloc_resource_args args;

 init_alloc_resource_args(, sizeof(args));
 args.memattr = VM_MEMATTR_WRITE_COMBINING;

 /* Uses WC for the implicit mapping. */
 res = bus_alloc_resource(, );

...

foobus_alloc_resource(..., union alloc_resource_args *args)
{
 union alloc_resource_args args2;

 switch (type) {
 case SYS_RES_IRQ:
 if (args == NULL) {
 init_alloc_resource_args(, sizeof(args2));
 args = 
 }
 /* Replace call to BUS_CONFIG_INTR on ACPI: */
 if (args->irq.polarity == INTR_POLARITY_CONFORMING &&
 device_has_polarity_from_CRS)
 args->irq.polarity = polarity_from_CRS;
 ...
}

However, you could associate arbitrary data with a resource request by
adding more members to the approriate struct in the union.


I like this idea. Mainly if we can add 'struct alloc_resource_args' into
'struct resource_list_entry' and, eventually, also into struct resource_i.
Inability to pass something more complex as single integer between bus
enumerator (aka resource_list_entry creator) and  bus_alloc_resource()
(aka resource_list_entry consumer) is serious limitation. At lest for me :)
Michal




Unfortunately, it doesn't actually work for resources that don't follow 
the bus hierarchy, however (see earlier follow-up emails to jhb from 
myself and others).

-Nathan

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Nathan Whitehorn]

On 07/23/16 03:45, Michal Meloun wrote:

Dne 21.07.2016 v 17:53 Nathan Whitehorn napsal(a):


On PowerPC, GENERIC64 supports FDT systems (some IBM hardware), OFW 
systems (Macs, some IBM hardware), systems with no device trees at 
all (old-style PS3), and systems with a mixture of device tree and no 
device tree (new-style PS3). On these, there is a mixture of "real" 
interrupts and GPIO-type interrupts. There is no limitation that this 
be used only for device-tree-type systems.


The system requires two things: an interrupt domain key and an 
arbitrary unique byte string describing the interrupt. When running 
with a device tree, these are set to the phandle of the 
interrupt-parent and the contents of the device tree interrupt 
specifier, respectively, and the system was of course developed with 
that in mind. But they don't need to be, and often aren't. You could 
make the domain an element of an enum (where "ACPI" is a choice, for 
instance -- this is what PS3 does), or set it to a pointer to a 
device_t, or really anything you like. Similarly, the interrupt 
specifier is totally free-form.


Yes, I agree. and i think that we followed the same direction. But i 
see two problems with this approach.
- in some cases (OFW, device_t)  you don't have  control over domain 
key value, so you cannot guarantee its uniqueness.

  Of course, probability of collision is low, but it is.


We could solve this in a number of ways, for example widening to 64 
bits, or adding another value (domain type, for example). You could also 
make an acpi_bus_map_intr() to go with the OFW one that connect in some 
machine-dependent code if you have fundamentally incompatible bus 
enumeration mechanisms that you expect to exist simultaneously -- but, 
of course, no systems like this seem to actually exist, so the problem 
is both easily solved and totally theoretical.


- within ofw_bus_map_intr() (or later - at the time when byte string 
must be decoded)  you are not able (easily) to differentiate
  between different formats, thus you are not able to select 
appropriate  decoder. The GPIO controller, for example,
  must be able to handle interrupts defined by standard OFW property, 
or by  pair concurrently.


In principle, you could solve that as above, or by registering a second 
interrupt domain for the same controller.


In practice, it doesn't matter since, in the GPIO case, for example, the 
GPIO controller is never itself also a normal interrupt controller (i.e. 
the GIC and GPIO controller are always different devices). As such, the 
theoretical does not occur in practice.


For this reason we makes domain key composite, in our model, the 
domain key consist of "domain key type"
and "domain key value". This composite key guarantees uniqueness and  
it also allows to select proper parser for byte string.


Yes, but this solves what is a nonexistant problem by making the system 
substantially less flexible and more invasive. Which is not a good tradeoff.



This is, imho, only one difference between us.


One of many, yes.



You could, for instance, set it to one of the structures introduced 
in r301453 if you wanted to.


I would have zero problems with changing the prototype of the 
existing dev/ofw function to something more generic in name, like:


bus_map_intr(device_t dev, uintptr_t iparent, size_t intrlen, void *intr)

instead of the existing equivalent:

ofw_bus_map_intr(device_t dev, phandle_t iparent, int icells, pcell_t 
*intr)


Our bus_map_intr() method is not indeed as replacement of 
ofw_bus_map_intr(). Its evolution of "how we can store more complex 
data to resource list (from bus enumerator) and transfer it  to 
bus_allocate_resource() and/or to bus_setup_intr()" in driver 
independent way. We found no reasonable way to do it, so we postponed 
reading of properties to bus_allocate_resource() time.


Right, but that is (a) a solved problem with ofw_bus_map_intr() and (b) 
this code doesn't solve it as completely. What does it let you do that 
the existing code does not? There has not been a single concrete example 
of something anyone wants to do on actual hardware so far in this 
discussion.


But now  jhb@ gives us alternative and I must say that this looks like 
a clean and elegant way how to make this (assuming that we can expand 
resource_list_entry by pointer to alloc_resource_args)


Except that jhb@'s suggestion doesn't actually work for interrupts for 
the reasons I and others have pointed out. We could make such a system, 
but it would be a different one.




By this, one byte string in OFW encoding can describe one IRQ and 
exactly same string in UEFI encoding can describe different IRQ.  
Or, in reverse, OFW and UEFI can describe same (and compatible) IRQ 
by two different strings.
This is exact reason, why we discards virtual IRQ idea and I think 
that this fact is root issue of this clash.
Probably it doesn't make sense to talk about others, unless we can 
find 

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Michal Meloun]

Dne 21.07.2016 v 23:35 John Baldwin napsal(a):
> On Thursday, July 21, 2016 01:37:42 PM Andrew Turner wrote:
>> On Wed, 20 Jul 2016 13:28:53 +0200
>> Michal Meloun  wrote:
>>> Dne 19.07.2016 v 17:06 Nathan Whitehorn napsal(a):
 2. It partially duplicates the functionality of OFW_BUS_MAP_INTR(),
 but is both problematically more general and less flexible (it has
 requirements on timing of PIC attachment vs. driver resource
 allocation)  
>>> OFW_BUS_MAP_INTR()  can parse only OFW  based data and expect that
>>> parsed data are magicaly stored within the call.
>>> The new method, bus_map_intr(),  can parse data from multiple sources 
>>> (OFW, UEFI / ACPI, synthetic[gpio device + pin number]).  It also
>>> returns parsed data back to caller.
>>> And no, it  doesn't  add any additional timing requirements .
>> I've been looking at ACPI on arm64. So far I have not found the need
>> for this with ACPI as we don't need to send the data to the interrupt
>> controller driver to be parsed in the way OFW/FDT needs to.
> ACPI though has a gross hack where we call BUS_CONFIG_INTR on the IRQ
> in bus_alloc_resource().  What I had advocated in the discussions
> leading up to this was to have some sort of opaque structure containing
> a set of properties (the sort of thing bus_map_resource and make_dev_s
> use) that was passed up at bus_setup_intr() time.  I think it should now
> be passed up at bus_alloc_resource() time instead, but it would allow bus
> drivers to "decorate" a SYS_RES_IRQ request as it goes up the device tree
> with properties that the interrupt controller can then associate with
> the IRQ cookie it allocates in its own code.  I would let the particular
> structure have different layouts for different resource types.  On x86 we
> would replace bus_config_intr by passing the level and trigger-mode in
> this structure.  However, I could also see allowing the memattr to be
> set for a SYS_RES_MEMORY resource so you could have a much shorter way
> than an explicit bus_map_resource to map an entire BAR as WC for example:
>
> struct alloc_resource_args {
> size_t len;
> union {
> struct {
> enum intr_trigger trigger;
> enum intr_polarity polarity;
> } irq;
> struct {
> vm_memattr_t memattr;
> } memory;
> }
> }
>
> ...
>
> union alloc_resource_args args;
>
> init_alloc_resource_args(, sizeof(args));
> args.memattr = VM_MEMATTR_WRITE_COMBINING;
>
> /* Uses WC for the implicit mapping. */
> res = bus_alloc_resource(, );
>
> ...
>
> foobus_alloc_resource(..., union alloc_resource_args *args)
> {
> union alloc_resource_args args2;
>
> switch (type) {
> case SYS_RES_IRQ:
> if (args == NULL) {
> init_alloc_resource_args(, sizeof(args2));
> args = 
> }
> /* Replace call to BUS_CONFIG_INTR on ACPI: */
> if (args->irq.polarity == INTR_POLARITY_CONFORMING &&
> device_has_polarity_from_CRS)
> args->irq.polarity = polarity_from_CRS;
> ...
> }
>
> However, you could associate arbitrary data with a resource request by
> adding more members to the approriate struct in the union.
>
I like this idea. Mainly if we can add 'struct alloc_resource_args' into
'struct resource_list_entry' and, eventually, also into struct resource_i.
Inability to pass something more complex as single integer between bus
enumerator (aka resource_list_entry creator) and  bus_alloc_resource()
(aka resource_list_entry consumer) is serious limitation. At lest for me :)
Michal


___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Michal Meloun]

Dne 21.07.2016 v 17:53 Nathan Whitehorn napsal(a):
>
>
> On 07/21/16 00:34, Michal Meloun wrote:
>> Dne 20.07.2016 v 17:45 Nathan Whitehorn napsal(a):
>>>
>>>
>>> On 07/20/16 04:28, Michal Meloun wrote:
 Dne 19.07.2016 v 17:06 Nathan Whitehorn napsal(a):
>
>
> On 07/19/16 04:13, Michal Meloun wrote:
>> Dne 19.07.2016 v 2:11 Nathan Whitehorn napsal(a):
>> Hi Nathan,
>> I’m afraid that skra is on vacation, for next 2 weeks (at
>> minimum), so
>> please don’t expect quick response.
>>
>>> Could you please describe what this change is in more detail?
>> Short description is appended.
>>
>>> It breaks a lot of encapsulations we have worked very hard to
>>> maintain,
>>> moves ARM code into MI parts of the kernel, and the OFW parts
>>> violate
>>> IEEE 1275 (the Open Firmware standard). In particular, there is no
>>> guarantee that the interrupts for a newbus (or OF) device are
>>> encoded in
>>> a property called "interrupts" (or, indeed, in any property at
>>> all) on
>>> that node and there are many, many device trees where that is
>>> not the
>>> case (e.g. ones with interrupt maps, as well as Apple hardware). By
>>> putting that knowledge into the OF root bus device, which we
>>> have tried
>>> to keep it out of, this enforces a standard that doesn't
>>> actually exist.
>> Imho, this patch doesn’t change anything in this area. Only
>> handling of
>> “interrupts” property is changed, all other cases are unchanged (I
>> hope).  Also, INTRNG code is currently shared by ARM, ARM64 and
>> MIPS.
>
> But "interrupts" isn't a generic part of OF. This makes it one,
> incorrectly.
 How? Can you be little more exact ?
>>>
>>> Because it puts knowledge into ofwbus that expects that children at
>>> arbitrary levels of nesting have interrupts defined by an
>>> "interrupts" property. You could patch this through on sub-devices,
>>> of course, but that's already done correctly by the existing
>>> ofw_bus_map_intr() code in a much more robust way that doesn't
>>> involve trying to guess how sub-buses and devices have chosen to
>>> allocate resources. Why reinvent the wheel all the way through the
>>> bus hierarchy?
>>  Nope, the code only expect that „interrupts" property is default way
>> hot to get interrupt description.  Any device or bus in the hierarchy
>> can fill appropriate resource list, or terminate call at any level.
>
> "interrupts" should not be the default -- it's part of the OF bindings
> for the bus and is used (notably) by simplebus. The issue of
> cross-correlating RIDs is a much larger problem, however.
>
Can we postpone this problem while, please?
> [snip]
>>>
>>
>> The patch simply postpones reading of interrupt property to
>> bus_alloc_resource() (called by consumer driver) time.
>>
>> Due to this, we can:
>> - parse  interrupt property. The interrupt driver must exist
>>at this time.
>
> This only works with some types of interrupt properties, not all,
> and breaks if the interrupt driver hasn't attached yet (which it
> can't be guaranteed to -- some PPC systems have interrupt drivers
> that live on the PCI bus, for example).
 How you can allocate (and reserve it in rman) interrupt if is not
 mapped (so you have not real IRQ number for it). Just for notice - 
 multiple virtual IRQs can be mapped into single real IRQ.
>>>
>>> The core idea is to think of the full interrupt specifier -- the
>>> interrupt parent and the full byte string in the device tree -- as
>>> the IRQ rather than the interrupt pin on some chip (which is
>>> usually, but not always, the first word in that byte string). The
>>> "virtual" IRQ number is just a compression of that longer piece of
>>> data, which usually can't fit in an rman resource.
>> I understand.  While this approach can works (and actually works) for
>> single sourced OFW world, it immediately fails if you must be able to
>> parse data from different sources (which uses different encoding)
>> (OFW, UEFI / ACPI, GPIO...) within one system.
>
>
> On PowerPC, GENERIC64 supports FDT systems (some IBM hardware), OFW
> systems (Macs, some IBM hardware), systems with no device trees at all
> (old-style PS3), and systems with a mixture of device tree and no
> device tree (new-style PS3). On these, there is a mixture of "real"
> interrupts and GPIO-type interrupts. There is no limitation that this
> be used only for device-tree-type systems.
>
> The system requires two things: an interrupt domain key and an
> arbitrary unique byte string describing the interrupt. When running
> with a device tree, these are set to the phandle of the
> interrupt-parent and the contents of the device tree interrupt
> specifier, respectively, and the system was of course developed with
> that in mind. But they don't need to be, and often aren't. You could
> make the domain an 

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Nathan Whitehorn]

On 07/22/16 13:53, Andrew Turner wrote:

On Fri, 22 Jul 2016 13:19:32 -0700
John Baldwin  wrote:


On Thursday, July 21, 2016 10:51:20 PM Nathan Whitehorn wrote:

On 07/21/16 14:35, John Baldwin wrote:

On Thursday, July 21, 2016 01:37:42 PM Andrew Turner wrote:

On Wed, 20 Jul 2016 13:28:53 +0200
Michal Meloun  wrote:

Dne 19.07.2016 v 17:06 Nathan Whitehorn napsal(a):

2. It partially duplicates the functionality of
OFW_BUS_MAP_INTR(), but is both problematically more general
and less flexible (it has requirements on timing of PIC
attachment vs. driver resource allocation)

OFW_BUS_MAP_INTR()  can parse only OFW  based data and expect
that parsed data are magicaly stored within the call.
The new method, bus_map_intr(),  can parse data from multiple
sources (OFW, UEFI / ACPI, synthetic[gpio device + pin
number]).  It also returns parsed data back to caller.
And no, it  doesn't  add any additional timing requirements .

I've been looking at ACPI on arm64. So far I have not found the
need for this with ACPI as we don't need to send the data to the
interrupt controller driver to be parsed in the way OFW/FDT
needs to.

ACPI though has a gross hack where we call BUS_CONFIG_INTR on the
IRQ in bus_alloc_resource().

I hadn't realized that. It looks like you could do essentially the
same thing we do on PowerPC to clean this up by explicitly mapping
the ACPI interrupt domains to different PICs with varying default
interrupt properties.
   

What I had advocated in the discussions
leading up to this was to have some sort of opaque structure
containing a set of properties (the sort of thing
bus_map_resource and make_dev_s use) that was passed up at
bus_setup_intr() time.
   

I think it should now
be passed up at bus_alloc_resource() time instead, but it would
allow bus drivers to "decorate" a SYS_RES_IRQ request as it goes
up the device tree with properties that the interrupt controller
can then associate with the IRQ cookie it allocates in its own
code.


We used to do this on PPC and MIPS, and the current code still
supports it, but it turned out not to be useful in the end for
IRQs. The hierarchy for IRQs rarely (read: almost never) follows
the bus hierarchy and often is enumerated in a different order. I
have hardware, for example, where the children of a single parent
bus are all wired to different interrupt controllers and sometimes
to a mixture of interrupt controllers. Those controllers are
cascaded in ways that cross the newbus tree laterally and, on some
of them, the parent device from the bus topology has interrupts
handled by its own (bus) children. Trying to make the newbus
parents do something sensible with all of this would be crazy and,
in the case where parents depend on resources provided by their own
children, impossible.

This is all to say that, since you want the interrupts to be
decorated along a path that usually has nothing to do with the
newbus hierarchy, it doesn't add much to add extra features to
resource allocation. ofw_bus_map_intr() is a newbus method to
support this kind of thing but, on all supported platforms, it is
implemented only in nexus and no cases have appeared where anyone
ever wanted anything at the intermediate layers.

Mmm.  Another idea that has been bandied about is to create a separate
"plane" in new-bus for an interrupt hierarchy and allow devices to
have "interrupt" parents that are not the same as the "bus" parent.
(Additional planes for power and clocks might also make sense.)  The
idea is borrowed from IOKit on Darwin which has multiple planes.  The
"bus" plane is always fully populated, but the other planes (Darwin
has one for power for example) can be sparse.  ACPI has methods that
effectively describe the power plane on x86.

For some planes the structure would look more like a directed
graph. Devices can have multiple clock parents, e.g. one for the
register interface, and another to drive the external bus.

I can also imagine the case where a device could have multiple
interrupt parents, an MMC/SD controller comes to mind where it may have
an interrupt on a GPIO line to detect the card being inserted, with
another for command completion, etc. In this case one parent would be
the standard interrupt controller, with the other being the GPIO
controller.

Andrew



That's a good point for the generalized system (this already works for 
the specific case of interrupts, of course). It's more the resources 
that have a non-bus parent than the device. There are also devices with 
multiple bus parents, but those are more oddball kinds of things (Apple 
made some on-board PCI devices that are also connected by a second 
non-PCI path).

-Nathan
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Andrew Turner]

On Fri, 22 Jul 2016 13:19:32 -0700
John Baldwin  wrote:

> On Thursday, July 21, 2016 10:51:20 PM Nathan Whitehorn wrote:
> > 
> > On 07/21/16 14:35, John Baldwin wrote:  
> > > On Thursday, July 21, 2016 01:37:42 PM Andrew Turner wrote:  
> > >> On Wed, 20 Jul 2016 13:28:53 +0200
> > >> Michal Meloun  wrote:  
> > >>> Dne 19.07.2016 v 17:06 Nathan Whitehorn napsal(a):  
> >  2. It partially duplicates the functionality of
> >  OFW_BUS_MAP_INTR(), but is both problematically more general
> >  and less flexible (it has requirements on timing of PIC
> >  attachment vs. driver resource allocation)  
> > >>> OFW_BUS_MAP_INTR()  can parse only OFW  based data and expect
> > >>> that parsed data are magicaly stored within the call.
> > >>> The new method, bus_map_intr(),  can parse data from multiple
> > >>> sources (OFW, UEFI / ACPI, synthetic[gpio device + pin
> > >>> number]).  It also returns parsed data back to caller.
> > >>> And no, it  doesn't  add any additional timing requirements .  
> > >> I've been looking at ACPI on arm64. So far I have not found the
> > >> need for this with ACPI as we don't need to send the data to the
> > >> interrupt controller driver to be parsed in the way OFW/FDT
> > >> needs to.  
> > > ACPI though has a gross hack where we call BUS_CONFIG_INTR on the
> > > IRQ in bus_alloc_resource().  
> > 
> > I hadn't realized that. It looks like you could do essentially the
> > same thing we do on PowerPC to clean this up by explicitly mapping
> > the ACPI interrupt domains to different PICs with varying default
> > interrupt properties.
> >   
> > > What I had advocated in the discussions
> > > leading up to this was to have some sort of opaque structure
> > > containing a set of properties (the sort of thing
> > > bus_map_resource and make_dev_s use) that was passed up at
> > > bus_setup_intr() time.  
> >   
> > > I think it should now
> > > be passed up at bus_alloc_resource() time instead, but it would
> > > allow bus drivers to "decorate" a SYS_RES_IRQ request as it goes
> > > up the device tree with properties that the interrupt controller
> > > can then associate with the IRQ cookie it allocates in its own
> > > code.  
> > 
> > 
> > We used to do this on PPC and MIPS, and the current code still
> > supports it, but it turned out not to be useful in the end for
> > IRQs. The hierarchy for IRQs rarely (read: almost never) follows
> > the bus hierarchy and often is enumerated in a different order. I
> > have hardware, for example, where the children of a single parent
> > bus are all wired to different interrupt controllers and sometimes
> > to a mixture of interrupt controllers. Those controllers are
> > cascaded in ways that cross the newbus tree laterally and, on some
> > of them, the parent device from the bus topology has interrupts
> > handled by its own (bus) children. Trying to make the newbus
> > parents do something sensible with all of this would be crazy and,
> > in the case where parents depend on resources provided by their own
> > children, impossible.
> > 
> > This is all to say that, since you want the interrupts to be
> > decorated along a path that usually has nothing to do with the
> > newbus hierarchy, it doesn't add much to add extra features to
> > resource allocation. ofw_bus_map_intr() is a newbus method to
> > support this kind of thing but, on all supported platforms, it is
> > implemented only in nexus and no cases have appeared where anyone
> > ever wanted anything at the intermediate layers.  
> 
> Mmm.  Another idea that has been bandied about is to create a separate
> "plane" in new-bus for an interrupt hierarchy and allow devices to
> have "interrupt" parents that are not the same as the "bus" parent.
> (Additional planes for power and clocks might also make sense.)  The
> idea is borrowed from IOKit on Darwin which has multiple planes.  The
> "bus" plane is always fully populated, but the other planes (Darwin
> has one for power for example) can be sparse.  ACPI has methods that
> effectively describe the power plane on x86.

For some planes the structure would look more like a directed
graph. Devices can have multiple clock parents, e.g. one for the
register interface, and another to drive the external bus.

I can also imagine the case where a device could have multiple
interrupt parents, an MMC/SD controller comes to mind where it may have
an interrupt on a GPIO line to detect the card being inserted, with
another for command completion, etc. In this case one parent would be
the standard interrupt controller, with the other being the GPIO
controller.

Andrew
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Nathan Whitehorn]

On 07/22/16 13:19, John Baldwin wrote:

On Thursday, July 21, 2016 10:51:20 PM Nathan Whitehorn wrote:

On 07/21/16 14:35, John Baldwin wrote:

On Thursday, July 21, 2016 01:37:42 PM Andrew Turner wrote:

On Wed, 20 Jul 2016 13:28:53 +0200
Michal Meloun  wrote:

Dne 19.07.2016 v 17:06 Nathan Whitehorn napsal(a):

2. It partially duplicates the functionality of OFW_BUS_MAP_INTR(),
but is both problematically more general and less flexible (it has
requirements on timing of PIC attachment vs. driver resource
allocation)

OFW_BUS_MAP_INTR()  can parse only OFW  based data and expect that
parsed data are magicaly stored within the call.
The new method, bus_map_intr(),  can parse data from multiple sources
(OFW, UEFI / ACPI, synthetic[gpio device + pin number]).  It also
returns parsed data back to caller.
And no, it  doesn't  add any additional timing requirements .

I've been looking at ACPI on arm64. So far I have not found the need
for this with ACPI as we don't need to send the data to the interrupt
controller driver to be parsed in the way OFW/FDT needs to.

ACPI though has a gross hack where we call BUS_CONFIG_INTR on the IRQ
in bus_alloc_resource().

I hadn't realized that. It looks like you could do essentially the same
thing we do on PowerPC to clean this up by explicitly mapping the ACPI
interrupt domains to different PICs with varying default interrupt
properties.


What I had advocated in the discussions
leading up to this was to have some sort of opaque structure containing
a set of properties (the sort of thing bus_map_resource and make_dev_s
use) that was passed up at bus_setup_intr() time.
I think it should now
be passed up at bus_alloc_resource() time instead, but it would allow bus
drivers to "decorate" a SYS_RES_IRQ request as it goes up the device tree
with properties that the interrupt controller can then associate with
the IRQ cookie it allocates in its own code.


We used to do this on PPC and MIPS, and the current code still supports
it, but it turned out not to be useful in the end for IRQs. The
hierarchy for IRQs rarely (read: almost never) follows the bus hierarchy
and often is enumerated in a different order. I have hardware, for
example, where the children of a single parent bus are all wired to
different interrupt controllers and sometimes to a mixture of interrupt
controllers. Those controllers are cascaded in ways that cross the
newbus tree laterally and, on some of them, the parent device from the
bus topology has interrupts handled by its own (bus) children. Trying to
make the newbus parents do something sensible with all of this would be
crazy and, in the case where parents depend on resources provided by
their own children, impossible.

This is all to say that, since you want the interrupts to be decorated
along a path that usually has nothing to do with the newbus hierarchy,
it doesn't add much to add extra features to resource allocation.
ofw_bus_map_intr() is a newbus method to support this kind of thing but,
on all supported platforms, it is implemented only in nexus and no cases
have appeared where anyone ever wanted anything at the intermediate layers.

Mmm.  Another idea that has been bandied about is to create a separate
"plane" in new-bus for an interrupt hierarchy and allow devices to have
"interrupt" parents that are not the same as the "bus" parent.  (Additional
planes for power and clocks might also make sense.)  The idea is borrowed
from IOKit on Darwin which has multiple planes.  The "bus" plane is always
fully populated, but the other planes (Darwin has one for power for example)
can be sparse.  ACPI has methods that effectively describe the power plane
on x86.



That's basically what the virtual IRQ code does: it implements a 
separate IRQ "plane" that is lazily interconnected during bus attachment 
and then strongly interconnected at configure_final() (i.e. it panics at 
that point if the topology hasn't converged yet). A generalized version 
of this concept would be useful for a number of other resource types 
(power, clocks, GPIOs, etc.) and I would be very happy to see it as a 
standard part of the OS.

-Nathan
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[John Baldwin]

On Thursday, July 21, 2016 10:51:20 PM Nathan Whitehorn wrote:
> 
> On 07/21/16 14:35, John Baldwin wrote:
> > On Thursday, July 21, 2016 01:37:42 PM Andrew Turner wrote:
> >> On Wed, 20 Jul 2016 13:28:53 +0200
> >> Michal Meloun  wrote:
> >>> Dne 19.07.2016 v 17:06 Nathan Whitehorn napsal(a):
>  2. It partially duplicates the functionality of OFW_BUS_MAP_INTR(),
>  but is both problematically more general and less flexible (it has
>  requirements on timing of PIC attachment vs. driver resource
>  allocation)
> >>> OFW_BUS_MAP_INTR()  can parse only OFW  based data and expect that
> >>> parsed data are magicaly stored within the call.
> >>> The new method, bus_map_intr(),  can parse data from multiple sources
> >>> (OFW, UEFI / ACPI, synthetic[gpio device + pin number]).  It also
> >>> returns parsed data back to caller.
> >>> And no, it  doesn't  add any additional timing requirements .
> >> I've been looking at ACPI on arm64. So far I have not found the need
> >> for this with ACPI as we don't need to send the data to the interrupt
> >> controller driver to be parsed in the way OFW/FDT needs to.
> > ACPI though has a gross hack where we call BUS_CONFIG_INTR on the IRQ
> > in bus_alloc_resource().
> 
> I hadn't realized that. It looks like you could do essentially the same 
> thing we do on PowerPC to clean this up by explicitly mapping the ACPI 
> interrupt domains to different PICs with varying default interrupt 
> properties.
> 
> > What I had advocated in the discussions
> > leading up to this was to have some sort of opaque structure containing
> > a set of properties (the sort of thing bus_map_resource and make_dev_s
> > use) that was passed up at bus_setup_intr() time.
> 
> > I think it should now
> > be passed up at bus_alloc_resource() time instead, but it would allow bus
> > drivers to "decorate" a SYS_RES_IRQ request as it goes up the device tree
> > with properties that the interrupt controller can then associate with
> > the IRQ cookie it allocates in its own code.
> 
> 
> We used to do this on PPC and MIPS, and the current code still supports 
> it, but it turned out not to be useful in the end for IRQs. The 
> hierarchy for IRQs rarely (read: almost never) follows the bus hierarchy 
> and often is enumerated in a different order. I have hardware, for 
> example, where the children of a single parent bus are all wired to 
> different interrupt controllers and sometimes to a mixture of interrupt 
> controllers. Those controllers are cascaded in ways that cross the 
> newbus tree laterally and, on some of them, the parent device from the 
> bus topology has interrupts handled by its own (bus) children. Trying to 
> make the newbus parents do something sensible with all of this would be 
> crazy and, in the case where parents depend on resources provided by 
> their own children, impossible.
> 
> This is all to say that, since you want the interrupts to be decorated 
> along a path that usually has nothing to do with the newbus hierarchy, 
> it doesn't add much to add extra features to resource allocation. 
> ofw_bus_map_intr() is a newbus method to support this kind of thing but, 
> on all supported platforms, it is implemented only in nexus and no cases 
> have appeared where anyone ever wanted anything at the intermediate layers.

Mmm.  Another idea that has been bandied about is to create a separate
"plane" in new-bus for an interrupt hierarchy and allow devices to have
"interrupt" parents that are not the same as the "bus" parent.  (Additional
planes for power and clocks might also make sense.)  The idea is borrowed
from IOKit on Darwin which has multiple planes.  The "bus" plane is always
fully populated, but the other planes (Darwin has one for power for example)
can be sparse.  ACPI has methods that effectively describe the power plane
on x86.

-- 
John Baldwin
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Nathan Whitehorn]

On 07/21/16 14:35, John Baldwin wrote:

On Thursday, July 21, 2016 01:37:42 PM Andrew Turner wrote:

On Wed, 20 Jul 2016 13:28:53 +0200
Michal Meloun  wrote:

Dne 19.07.2016 v 17:06 Nathan Whitehorn napsal(a):

2. It partially duplicates the functionality of OFW_BUS_MAP_INTR(),
but is both problematically more general and less flexible (it has
requirements on timing of PIC attachment vs. driver resource
allocation)

OFW_BUS_MAP_INTR()  can parse only OFW  based data and expect that
parsed data are magicaly stored within the call.
The new method, bus_map_intr(),  can parse data from multiple sources
(OFW, UEFI / ACPI, synthetic[gpio device + pin number]).  It also
returns parsed data back to caller.
And no, it  doesn't  add any additional timing requirements .

I've been looking at ACPI on arm64. So far I have not found the need
for this with ACPI as we don't need to send the data to the interrupt
controller driver to be parsed in the way OFW/FDT needs to.

ACPI though has a gross hack where we call BUS_CONFIG_INTR on the IRQ
in bus_alloc_resource().


I hadn't realized that. It looks like you could do essentially the same 
thing we do on PowerPC to clean this up by explicitly mapping the ACPI 
interrupt domains to different PICs with varying default interrupt 
properties.



What I had advocated in the discussions
leading up to this was to have some sort of opaque structure containing
a set of properties (the sort of thing bus_map_resource and make_dev_s
use) that was passed up at bus_setup_intr() time.



I think it should now
be passed up at bus_alloc_resource() time instead, but it would allow bus
drivers to "decorate" a SYS_RES_IRQ request as it goes up the device tree
with properties that the interrupt controller can then associate with
the IRQ cookie it allocates in its own code.



We used to do this on PPC and MIPS, and the current code still supports 
it, but it turned out not to be useful in the end for IRQs. The 
hierarchy for IRQs rarely (read: almost never) follows the bus hierarchy 
and often is enumerated in a different order. I have hardware, for 
example, where the children of a single parent bus are all wired to 
different interrupt controllers and sometimes to a mixture of interrupt 
controllers. Those controllers are cascaded in ways that cross the 
newbus tree laterally and, on some of them, the parent device from the 
bus topology has interrupts handled by its own (bus) children. Trying to 
make the newbus parents do something sensible with all of this would be 
crazy and, in the case where parents depend on resources provided by 
their own children, impossible.


This is all to say that, since you want the interrupts to be decorated 
along a path that usually has nothing to do with the newbus hierarchy, 
it doesn't add much to add extra features to resource allocation. 
ofw_bus_map_intr() is a newbus method to support this kind of thing but, 
on all supported platforms, it is implemented only in nexus and no cases 
have appeared where anyone ever wanted anything at the intermediate layers.



I would let the particular
structure have different layouts for different resource types.  On x86 we
would replace bus_config_intr by passing the level and trigger-mode in
this structure.  However, I could also see allowing the memattr to be
set for a SYS_RES_MEMORY resource so you could have a much shorter way
than an explicit bus_map_resource to map an entire BAR as WC for example:

 struct alloc_resource_args {
 size_t len;
 union {
 struct {
 enum intr_trigger trigger;
 enum intr_polarity polarity;
 } irq;
 struct {
 vm_memattr_t memattr;
 } memory;
 }
 }

...

 union alloc_resource_args args;

 init_alloc_resource_args(, sizeof(args));
 args.memattr = VM_MEMATTR_WRITE_COMBINING;

 /* Uses WC for the implicit mapping. */
 res = bus_alloc_resource(, );

...

foobus_alloc_resource(..., union alloc_resource_args *args)
{
 union alloc_resource_args args2;

 switch (type) {
 case SYS_RES_IRQ:
 if (args == NULL) {
 init_alloc_resource_args(, sizeof(args2));
 args = 
 }
 /* Replace call to BUS_CONFIG_INTR on ACPI: */
 if (args->irq.polarity == INTR_POLARITY_CONFORMING &&
 device_has_polarity_from_CRS)
 args->irq.polarity = polarity_from_CRS;
 ...
}

However, you could associate arbitrary data with a resource request by
adding more members to the approriate struct in the union.



For memory, I think this is an interesting concept, but it really 
doesn't match well with what you would want to do for interrupts or for, 
say, GPIOs in which the lines of control are fundamentally unrelated to 
the newbus hierarchy.

-Nathan
___
svn-src-all@freebsd.org 

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[John Baldwin]

On Thursday, July 21, 2016 01:37:42 PM Andrew Turner wrote:
> On Wed, 20 Jul 2016 13:28:53 +0200
> Michal Meloun  wrote:
> > Dne 19.07.2016 v 17:06 Nathan Whitehorn napsal(a):
> > > 2. It partially duplicates the functionality of OFW_BUS_MAP_INTR(),
> > > but is both problematically more general and less flexible (it has
> > > requirements on timing of PIC attachment vs. driver resource
> > > allocation)  
> > OFW_BUS_MAP_INTR()  can parse only OFW  based data and expect that
> > parsed data are magicaly stored within the call.
> > The new method, bus_map_intr(),  can parse data from multiple sources 
> > (OFW, UEFI / ACPI, synthetic[gpio device + pin number]).  It also
> > returns parsed data back to caller.
> > And no, it  doesn't  add any additional timing requirements .
> 
> I've been looking at ACPI on arm64. So far I have not found the need
> for this with ACPI as we don't need to send the data to the interrupt
> controller driver to be parsed in the way OFW/FDT needs to.

ACPI though has a gross hack where we call BUS_CONFIG_INTR on the IRQ
in bus_alloc_resource().  What I had advocated in the discussions
leading up to this was to have some sort of opaque structure containing
a set of properties (the sort of thing bus_map_resource and make_dev_s
use) that was passed up at bus_setup_intr() time.  I think it should now
be passed up at bus_alloc_resource() time instead, but it would allow bus
drivers to "decorate" a SYS_RES_IRQ request as it goes up the device tree
with properties that the interrupt controller can then associate with
the IRQ cookie it allocates in its own code.  I would let the particular
structure have different layouts for different resource types.  On x86 we
would replace bus_config_intr by passing the level and trigger-mode in
this structure.  However, I could also see allowing the memattr to be
set for a SYS_RES_MEMORY resource so you could have a much shorter way
than an explicit bus_map_resource to map an entire BAR as WC for example:

struct alloc_resource_args {
size_t len;
union {
struct {
enum intr_trigger trigger;
enum intr_polarity polarity;
} irq;
struct {
vm_memattr_t memattr;
} memory;
}
}

...

union alloc_resource_args args;

init_alloc_resource_args(, sizeof(args));
args.memattr = VM_MEMATTR_WRITE_COMBINING;

/* Uses WC for the implicit mapping. */
res = bus_alloc_resource(, );

...

foobus_alloc_resource(..., union alloc_resource_args *args)
{
union alloc_resource_args args2;

switch (type) {
case SYS_RES_IRQ:
if (args == NULL) {
init_alloc_resource_args(, sizeof(args2));
args = 
}
/* Replace call to BUS_CONFIG_INTR on ACPI: */
if (args->irq.polarity == INTR_POLARITY_CONFORMING &&
device_has_polarity_from_CRS)
args->irq.polarity = polarity_from_CRS;
...
}

However, you could associate arbitrary data with a resource request by
adding more members to the approriate struct in the union.

-- 
John Baldwin
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Nathan Whitehorn]

On 07/21/16 00:34, Michal Meloun wrote:

Dne 20.07.2016 v 17:45 Nathan Whitehorn napsal(a):



On 07/20/16 04:28, Michal Meloun wrote:

Dne 19.07.2016 v 17:06 Nathan Whitehorn napsal(a):



On 07/19/16 04:13, Michal Meloun wrote:

Dne 19.07.2016 v 2:11 Nathan Whitehorn napsal(a):
Hi Nathan,
I’m afraid that skra is on vacation, for next 2 weeks (at 
minimum), so

please don’t expect quick response.


Could you please describe what this change is in more detail?

Short description is appended.

It breaks a lot of encapsulations we have worked very hard to 
maintain,
moves ARM code into MI parts of the kernel, and the OFW parts 
violate

IEEE 1275 (the Open Firmware standard). In particular, there is no
guarantee that the interrupts for a newbus (or OF) device are 
encoded in
a property called "interrupts" (or, indeed, in any property at 
all) on
that node and there are many, many device trees where that is not 
the

case (e.g. ones with interrupt maps, as well as Apple hardware). By
putting that knowledge into the OF root bus device, which we have 
tried
to keep it out of, this enforces a standard that doesn't actually 
exist.
Imho, this patch doesn’t change anything in this area. Only 
handling of

“interrupts” property is changed, all other cases are unchanged (I
hope).  Also, INTRNG code is currently shared by ARM, ARM64 and MIPS.


But "interrupts" isn't a generic part of OF. This makes it one, 
incorrectly.

How? Can you be little more exact ?


Because it puts knowledge into ofwbus that expects that children at 
arbitrary levels of nesting have interrupts defined by an 
"interrupts" property. You could patch this through on sub-devices, 
of course, but that's already done correctly by the existing 
ofw_bus_map_intr() code in a much more robust way that doesn't 
involve trying to guess how sub-buses and devices have chosen to 
allocate resources. Why reinvent the wheel all the way through the 
bus hierarchy?
 Nope, the code only expect that „interrupts" property is default way 
hot to get interrupt description.  Any device or bus in the hierarchy 
can fill appropriate resource list, or terminate call at any level.


"interrupts" should not be the default -- it's part of the OF bindings 
for the bus and is used (notably) by simplebus. The issue of 
cross-correlating RIDs is a much larger problem, however.


[snip]




The patch simply postpones reading of interrupt property to
bus_alloc_resource() (called by consumer driver) time.

Due to this, we can:
- parse  interrupt property. The interrupt driver must exist
   at this time.


This only works with some types of interrupt properties, not all, 
and breaks if the interrupt driver hasn't attached yet (which it 
can't be guaranteed to -- some PPC systems have interrupt drivers 
that live on the PCI bus, for example).
How you can allocate (and reserve it in rman) interrupt if is not 
mapped (so you have not real IRQ number for it). Just for notice -  
multiple virtual IRQs can be mapped into single real IRQ.


The core idea is to think of the full interrupt specifier -- the 
interrupt parent and the full byte string in the device tree -- as 
the IRQ rather than the interrupt pin on some chip (which is usually, 
but not always, the first word in that byte string). The "virtual" 
IRQ number is just a compression of that longer piece of data, which 
usually can't fit in an rman resource.
I understand.  While this approach can works (and actually works) for 
single sourced OFW world, it immediately fails if you must be able to 
parse data from different sources (which uses different encoding) 
(OFW, UEFI / ACPI, GPIO...) within one system.



On PowerPC, GENERIC64 supports FDT systems (some IBM hardware), OFW 
systems (Macs, some IBM hardware), systems with no device trees at all 
(old-style PS3), and systems with a mixture of device tree and no device 
tree (new-style PS3). On these, there is a mixture of "real" interrupts 
and GPIO-type interrupts. There is no limitation that this be used only 
for device-tree-type systems.


The system requires two things: an interrupt domain key and an arbitrary 
unique byte string describing the interrupt. When running with a device 
tree, these are set to the phandle of the interrupt-parent and the 
contents of the device tree interrupt specifier, respectively, and the 
system was of course developed with that in mind. But they don't need to 
be, and often aren't. You could make the domain an element of an enum 
(where "ACPI" is a choice, for instance -- this is what PS3 does), or 
set it to a pointer to a device_t, or really anything you like. 
Similarly, the interrupt specifier is totally free-form. You could, for 
instance, set it to one of the structures introduced in r301453 if you 
wanted to.


I would have zero problems with changing the prototype of the existing 
dev/ofw function to something more generic in name, like:


bus_map_intr(device_t dev, uintptr_t iparent, size_t intrlen, void *intr)

instead of 

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Jonathan T. Looney]

On 7/21/16, 3:34 AM, "owner-src-committ...@freebsd.org on behalf of Michal 
Meloun"  wrote:

> The API is part of still unstable, experimental INTRNG, so its not fixed we 
> we can 
> change it at any time, I think.

If INTRNG is unstable and experimental, why is it in the GENERIC kernel for 
arm64? Or, is the entire arm64 architecture considered unstable and 
experimental in 11.0?

(I'm not trying to take sides; these are honest questions.)

Jonathan


___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Andrew Turner]

On Wed, 20 Jul 2016 13:28:53 +0200
Michal Meloun  wrote:
> Dne 19.07.2016 v 17:06 Nathan Whitehorn napsal(a):
> > 2. It partially duplicates the functionality of OFW_BUS_MAP_INTR(),
> > but is both problematically more general and less flexible (it has
> > requirements on timing of PIC attachment vs. driver resource
> > allocation)  
> OFW_BUS_MAP_INTR()  can parse only OFW  based data and expect that
> parsed data are magicaly stored within the call.
> The new method, bus_map_intr(),  can parse data from multiple sources 
> (OFW, UEFI / ACPI, synthetic[gpio device + pin number]).  It also
> returns parsed data back to caller.
> And no, it  doesn't  add any additional timing requirements .

I've been looking at ACPI on arm64. So far I have not found the need
for this with ACPI as we don't need to send the data to the interrupt
controller driver to be parsed in the way OFW/FDT needs to.

Andrew
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Michal Meloun]

Dne 20.07.2016 v 17:45 Nathan Whitehorn napsal(a):
>
>
> On 07/20/16 04:28, Michal Meloun wrote:
>> Dne 19.07.2016 v 17:06 Nathan Whitehorn napsal(a):
>>>
>>>
>>> On 07/19/16 04:13, Michal Meloun wrote:
 Dne 19.07.2016 v 2:11 Nathan Whitehorn napsal(a):
 Hi Nathan,
 I’m afraid that skra is on vacation, for next 2 weeks (at minimum), so
 please don’t expect quick response.

> Could you please describe what this change is in more detail?
 Short description is appended.

> It breaks a lot of encapsulations we have worked very hard to
> maintain,
> moves ARM code into MI parts of the kernel, and the OFW parts violate
> IEEE 1275 (the Open Firmware standard). In particular, there is no
> guarantee that the interrupts for a newbus (or OF) device are
> encoded in
> a property called "interrupts" (or, indeed, in any property at
> all) on
> that node and there are many, many device trees where that is not the
> case (e.g. ones with interrupt maps, as well as Apple hardware). By
> putting that knowledge into the OF root bus device, which we have
> tried
> to keep it out of, this enforces a standard that doesn't actually
> exist.
 Imho, this patch doesn’t change anything in this area. Only
 handling of
 “interrupts” property is changed, all other cases are unchanged (I
 hope).  Also, INTRNG code is currently shared by ARM, ARM64 and MIPS.
>>>
>>> But "interrupts" isn't a generic part of OF. This makes it one,
>>> incorrectly.
>> How? Can you be little more exact ?
>
> Because it puts knowledge into ofwbus that expects that children at
> arbitrary levels of nesting have interrupts defined by an "interrupts"
> property. You could patch this through on sub-devices, of course, but
> that's already done correctly by the existing ofw_bus_map_intr() code
> in a much more robust way that doesn't involve trying to guess how
> sub-buses and devices have chosen to allocate resources. Why reinvent
> the wheel all the way through the bus hierarchy?
 Nope, the code only expect that „interrupts" property is default way
hot to get interrupt description.  Any device or bus in the hierarchy
can fill appropriate resource list, or terminate call at any level.
>
>>>

> I'm hesitant to ask for reversion on something that landed 6 weeks
> ago
> without me noticing, but this needs a lot more architectural work
> before
> any parts of the kernel should use it.
> -Nathan
 I think that it’s too late.  This patch series consist of r301451
 (https://reviews.freebsd.org/D6632),
 r301453, r301539 and 301543.  And new GPIO interrupts are currently
 used
 (by in tree drivers or in development trees).
>>>
>>> Well, then we need in-place rearchitecture.
>>>


 The root of problem is that standard way of delivering interrupt
 resource to consumer driver doesn’t works in OFW world.

 So we have some fact:
 - the format of interrupt property is dependent of interrupt
controller and only interrupt controller can parse it.
 - the interrupt property can have more data than just interrupt
number.
 - single interrupt controller must be able to handle multiple
format of interrupt description.

 In pre-patchset era, simplebus enumerates children and attempts to set
 memory and interrupts to resource list for them. But the interrupt
 controllers are not yet populated so nobody can parse interrupt
 property. Moreover, in all cases (parsed or not), we cannot store
 complete interrupt description into resource list.
>>>
>>> We have done this for many years on PowerPC and sparc64 with delayed
>>> configuration of interrupts and a look-up table. This handles
>>> complicated bus configurations better than this code and requires no
>>> changes outside of a few MD files. That is why the (now partially
>>> duplicated) OFW_BUS_MAP_INTR() function exists. That one also has
>>> the benefit of still working when used in conjunction with, e.g.,
>>> devices with an interrupt-map-mask property.
>>>

 The patch simply postpones reading of interrupt property to
 bus_alloc_resource() (called by consumer driver) time.

 Due to this, we can:
 - parse  interrupt property. The interrupt driver must exist
at this time.
>>>
>>> This only works with some types of interrupt properties, not all,
>>> and breaks if the interrupt driver hasn't attached yet (which it
>>> can't be guaranteed to -- some PPC systems have interrupt drivers
>>> that live on the PCI bus, for example).
>> How you can allocate (and reserve it in rman) interrupt if is not
>> mapped (so you have not real IRQ number for it). Just for notice - 
>> multiple virtual IRQs can be mapped into single real IRQ.
>
> The core idea is to think of the full interrupt specifier -- the
> interrupt parent and the full byte string in the device tree -- as the
> IRQ rather 

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Nathan Whitehorn]

On 07/20/16 04:28, Michal Meloun wrote:

Dne 19.07.2016 v 17:06 Nathan Whitehorn napsal(a):



On 07/19/16 04:13, Michal Meloun wrote:

Dne 19.07.2016 v 2:11 Nathan Whitehorn napsal(a):
Hi Nathan,
I’m afraid that skra is on vacation, for next 2 weeks (at minimum), so
please don’t expect quick response.


Could you please describe what this change is in more detail?

Short description is appended.

It breaks a lot of encapsulations we have worked very hard to 
maintain,

moves ARM code into MI parts of the kernel, and the OFW parts violate
IEEE 1275 (the Open Firmware standard). In particular, there is no
guarantee that the interrupts for a newbus (or OF) device are 
encoded in

a property called "interrupts" (or, indeed, in any property at all) on
that node and there are many, many device trees where that is not the
case (e.g. ones with interrupt maps, as well as Apple hardware). By
putting that knowledge into the OF root bus device, which we have 
tried
to keep it out of, this enforces a standard that doesn't actually 
exist.

Imho, this patch doesn’t change anything in this area. Only handling of
“interrupts” property is changed, all other cases are unchanged (I
hope).  Also, INTRNG code is currently shared by ARM, ARM64 and MIPS.


But "interrupts" isn't a generic part of OF. This makes it one, 
incorrectly.

How? Can you be little more exact ?


Because it puts knowledge into ofwbus that expects that children at 
arbitrary levels of nesting have interrupts defined by an "interrupts" 
property. You could patch this through on sub-devices, of course, but 
that's already done correctly by the existing ofw_bus_map_intr() code in 
a much more robust way that doesn't involve trying to guess how 
sub-buses and devices have chosen to allocate resources. Why reinvent 
the wheel all the way through the bus hierarchy?







I'm hesitant to ask for reversion on something that landed 6 weeks ago
without me noticing, but this needs a lot more architectural work 
before

any parts of the kernel should use it.
-Nathan

I think that it’s too late.  This patch series consist of r301451
(https://reviews.freebsd.org/D6632),
r301453, r301539 and 301543.  And new GPIO interrupts are currently 
used

(by in tree drivers or in development trees).


Well, then we need in-place rearchitecture.




The root of problem is that standard way of delivering interrupt
resource to consumer driver doesn’t works in OFW world.

So we have some fact:
- the format of interrupt property is dependent of interrupt
   controller and only interrupt controller can parse it.
- the interrupt property can have more data than just interrupt
   number.
- single interrupt controller must be able to handle multiple
   format of interrupt description.

In pre-patchset era, simplebus enumerates children and attempts to set
memory and interrupts to resource list for them. But the interrupt
controllers are not yet populated so nobody can parse interrupt
property. Moreover, in all cases (parsed or not), we cannot store
complete interrupt description into resource list.


We have done this for many years on PowerPC and sparc64 with delayed 
configuration of interrupts and a look-up table. This handles 
complicated bus configurations better than this code and requires no 
changes outside of a few MD files. That is why the (now partially 
duplicated) OFW_BUS_MAP_INTR() function exists. That one also has the 
benefit of still working when used in conjunction with, e.g., devices 
with an interrupt-map-mask property.




The patch simply postpones reading of interrupt property to
bus_alloc_resource() (called by consumer driver) time.

Due to this, we can:
- parse  interrupt property. The interrupt driver must exist
   at this time.


This only works with some types of interrupt properties, not all, and 
breaks if the interrupt driver hasn't attached yet (which it can't be 
guaranteed to -- some PPC systems have interrupt drivers that live on 
the PCI bus, for example).
How you can allocate (and reserve it in rman) interrupt if is not 
mapped (so you have not real IRQ number for it). Just for notice -  
multiple virtual IRQs can be mapped into single real IRQ.


The core idea is to think of the full interrupt specifier -- the 
interrupt parent and the full byte string in the device tree -- as the 
IRQ rather than the interrupt pin on some chip (which is usually, but 
not always, the first word in that byte string). The "virtual" IRQ 
number is just a compression of that longer piece of data, which usually 
can't fit in an rman resource.


There is no need to actually activate those interrupts before interrupts 
are enabled, so you can just cache them in a table until the end of 
device probing, which lets you break circular dependency loops between 
bus and interrupt topology.


So long as you keep track of your mapping and the same (parent, 
interrupt specifier) parent always gives the same virtual IRQ, there is 
no way in this system to map multiple active 

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Michal Meloun]

Dne 19.07.2016 v 17:06 Nathan Whitehorn napsal(a):
>
>
> On 07/19/16 04:13, Michal Meloun wrote:
>> Dne 19.07.2016 v 2:11 Nathan Whitehorn napsal(a):
>> Hi Nathan,
>> I’m afraid that skra is on vacation, for next 2 weeks (at minimum), so
>> please don’t expect quick response.
>>
>>> Could you please describe what this change is in more detail?
>> Short description is appended.
>>
>>> It breaks a lot of encapsulations we have worked very hard to maintain,
>>> moves ARM code into MI parts of the kernel, and the OFW parts violate
>>> IEEE 1275 (the Open Firmware standard). In particular, there is no
>>> guarantee that the interrupts for a newbus (or OF) device are
>>> encoded in
>>> a property called "interrupts" (or, indeed, in any property at all) on
>>> that node and there are many, many device trees where that is not the
>>> case (e.g. ones with interrupt maps, as well as Apple hardware). By
>>> putting that knowledge into the OF root bus device, which we have tried
>>> to keep it out of, this enforces a standard that doesn't actually
>>> exist.
>> Imho, this patch doesn’t change anything in this area. Only handling of
>> “interrupts” property is changed, all other cases are unchanged (I
>> hope).  Also, INTRNG code is currently shared by ARM, ARM64 and MIPS.
>
> But "interrupts" isn't a generic part of OF. This makes it one,
> incorrectly.
How? Can you be little more exact ?
>
>>
>>> I'm hesitant to ask for reversion on something that landed 6 weeks ago
>>> without me noticing, but this needs a lot more architectural work
>>> before
>>> any parts of the kernel should use it.
>>> -Nathan
>> I think that it’s too late.  This patch series consist of r301451
>> (https://reviews.freebsd.org/D6632),
>> r301453, r301539 and 301543.  And new GPIO interrupts are currently used
>> (by in tree drivers or in development trees).
>
> Well, then we need in-place rearchitecture.
>
>>
>>
>> The root of problem is that standard way of delivering interrupt
>> resource to consumer driver doesn’t works in OFW world.
>>
>> So we have some fact:
>> - the format of interrupt property is dependent of interrupt
>>controller and only interrupt controller can parse it.
>> - the interrupt property can have more data than just interrupt
>>number.
>> - single interrupt controller must be able to handle multiple
>>format of interrupt description.
>>
>> In pre-patchset era, simplebus enumerates children and attempts to set
>> memory and interrupts to resource list for them. But the interrupt
>> controllers are not yet populated so nobody can parse interrupt
>> property. Moreover, in all cases (parsed or not), we cannot store
>> complete interrupt description into resource list.
>
> We have done this for many years on PowerPC and sparc64 with delayed
> configuration of interrupts and a look-up table. This handles
> complicated bus configurations better than this code and requires no
> changes outside of a few MD files. That is why the (now partially
> duplicated) OFW_BUS_MAP_INTR() function exists. That one also has the
> benefit of still working when used in conjunction with, e.g., devices
> with an interrupt-map-mask property.
>
>>
>> The patch simply postpones reading of interrupt property to
>> bus_alloc_resource() (called by consumer driver) time.
>>
>> Due to this, we can:
>> - parse  interrupt property. The interrupt driver must exist
>>at this time.
>
> This only works with some types of interrupt properties, not all, and
> breaks if the interrupt driver hasn't attached yet (which it can't be
> guaranteed to -- some PPC systems have interrupt drivers that live on
> the PCI bus, for example).
How you can allocate (and reserve it in rman) interrupt if is not mapped
(so you have not real IRQ number for it). Just for notice -  multiple
virtual IRQs can be mapped into single real IRQ.

>
>> - bus_alloc_resource() returns resource, so we can attach parsed
>>interrupt data to it. By this, the resource itself can be used
>>for delivering configuration data to subsequent call to
>>bus_setup_intr() (or to all related  bus_() calls).
>>
>>
>> The patched code still accepts delivering of interrupts in resource
>> list.
>>
>> Michal
>>
>
> Given that other code depends on this, fixing it will likely require
> some complex work. I wish I had known about it when it went in.
>
> There are three main problems:
> 1. It doesn't work for interrupts defined by other mechanisms (e.g.
> interrupt-map properties)
I aggree, but missing ' interrupt-map' functioanlity is not caused by
this patch.
 
> 2. It partially duplicates the functionality of OFW_BUS_MAP_INTR(),
> but is both problematically more general and less flexible (it has
> requirements on timing of PIC attachment vs. driver resource allocation)
OFW_BUS_MAP_INTR()  can parse only OFW  based data and expect that
parsed data are magicaly stored within the call.
The new method, bus_map_intr(),  can parse data from multiple sources 
(OFW, UEFI / ACPI, synthetic[gpio 

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Nathan Whitehorn]

On 07/19/16 04:13, Michal Meloun wrote:

Dne 19.07.2016 v 2:11 Nathan Whitehorn napsal(a):
Hi Nathan,
I’m afraid that skra is on vacation, for next 2 weeks (at minimum), so
please don’t expect quick response.


Could you please describe what this change is in more detail?

Short description is appended.


It breaks a lot of encapsulations we have worked very hard to maintain,
moves ARM code into MI parts of the kernel, and the OFW parts violate
IEEE 1275 (the Open Firmware standard). In particular, there is no
guarantee that the interrupts for a newbus (or OF) device are encoded in
a property called "interrupts" (or, indeed, in any property at all) on
that node and there are many, many device trees where that is not the
case (e.g. ones with interrupt maps, as well as Apple hardware). By
putting that knowledge into the OF root bus device, which we have tried
to keep it out of, this enforces a standard that doesn't actually exist.

Imho, this patch doesn’t change anything in this area. Only handling of
“interrupts” property is changed, all other cases are unchanged (I
hope).  Also, INTRNG code is currently shared by ARM, ARM64 and MIPS.


I'm hesitant to ask for reversion on something that landed 6 weeks ago
without me noticing, but this needs a lot more architectural work before
any parts of the kernel should use it.
-Nathan

I think that it’s too late.  This patch series consist of r301451
(https://reviews.freebsd.org/D6632),
r301453, r301539 and 301543.  And new GPIO interrupts are currently used
(by in tree drivers or in development trees).




[See other email for information for detail on why I am concerned about 
this code]


Looking through those commits and the current state of HEAD, it turns 
out bus_map_intr() is not yet used anywhere in the tree; there is just a 
stub implementation in dev/ofw/ofwbus.c and a prototype that is never 
called in sys/bus.h. As such, I would in fact like to ask for the 
reversion of both r301451 and r301453 at this point, especially from the 
stable/11 branch. Adding a new API is something we can do to stable/11 
at any point; removing one, or changing one, is something we cannot do.


After that, I am more than happy to help move ARM code to use the 
existing dev/ofw scheme for handling interrupt metadata.

-Nathan
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Nathan Whitehorn]

On 07/19/16 04:13, Michal Meloun wrote:

Dne 19.07.2016 v 2:11 Nathan Whitehorn napsal(a):
Hi Nathan,
I’m afraid that skra is on vacation, for next 2 weeks (at minimum), so
please don’t expect quick response.


Could you please describe what this change is in more detail?

Short description is appended.


It breaks a lot of encapsulations we have worked very hard to maintain,
moves ARM code into MI parts of the kernel, and the OFW parts violate
IEEE 1275 (the Open Firmware standard). In particular, there is no
guarantee that the interrupts for a newbus (or OF) device are encoded in
a property called "interrupts" (or, indeed, in any property at all) on
that node and there are many, many device trees where that is not the
case (e.g. ones with interrupt maps, as well as Apple hardware). By
putting that knowledge into the OF root bus device, which we have tried
to keep it out of, this enforces a standard that doesn't actually exist.

Imho, this patch doesn’t change anything in this area. Only handling of
“interrupts” property is changed, all other cases are unchanged (I
hope).  Also, INTRNG code is currently shared by ARM, ARM64 and MIPS.


But "interrupts" isn't a generic part of OF. This makes it one, incorrectly.




I'm hesitant to ask for reversion on something that landed 6 weeks ago
without me noticing, but this needs a lot more architectural work before
any parts of the kernel should use it.
-Nathan

I think that it’s too late.  This patch series consist of r301451
(https://reviews.freebsd.org/D6632),
r301453, r301539 and 301543.  And new GPIO interrupts are currently used
(by in tree drivers or in development trees).


Well, then we need in-place rearchitecture.




The root of problem is that standard way of delivering interrupt
resource to consumer driver doesn’t works in OFW world.

So we have some fact:
- the format of interrupt property is dependent of interrupt
   controller and only interrupt controller can parse it.
- the interrupt property can have more data than just interrupt
   number.
- single interrupt controller must be able to handle multiple
   format of interrupt description.

In pre-patchset era, simplebus enumerates children and attempts to set
memory and interrupts to resource list for them. But the interrupt
controllers are not yet populated so nobody can parse interrupt
property. Moreover, in all cases (parsed or not), we cannot store
complete interrupt description into resource list.


We have done this for many years on PowerPC and sparc64 with delayed 
configuration of interrupts and a look-up table. This handles 
complicated bus configurations better than this code and requires no 
changes outside of a few MD files. That is why the (now partially 
duplicated) OFW_BUS_MAP_INTR() function exists. That one also has the 
benefit of still working when used in conjunction with, e.g., devices 
with an interrupt-map-mask property.




The patch simply postpones reading of interrupt property to
bus_alloc_resource() (called by consumer driver) time.

Due to this, we can:
- parse  interrupt property. The interrupt driver must exist
   at this time.


This only works with some types of interrupt properties, not all, and 
breaks if the interrupt driver hasn't attached yet (which it can't be 
guaranteed to -- some PPC systems have interrupt drivers that live on 
the PCI bus, for example).



- bus_alloc_resource() returns resource, so we can attach parsed
   interrupt data to it. By this, the resource itself can be used
   for delivering configuration data to subsequent call to
   bus_setup_intr() (or to all related  bus_() calls).


The patched code still accepts delivering of interrupts in resource list.

Michal



Given that other code depends on this, fixing it will likely require 
some complex work. I wish I had known about it when it went in.


There are three main problems:
1. It doesn't work for interrupts defined by other mechanisms (e.g. 
interrupt-map properties)
2. It partially duplicates the functionality of OFW_BUS_MAP_INTR(), but 
is both problematically more general and less flexible (it has 
requirements on timing of PIC attachment vs. driver resource allocation)
3. It is not fully transparent to end code. Since it happens at 
bus_alloc_resource() time, it is complicated to get the appropriate 
values for IRQs constructed by composite techniques (interrupt-map vs. 
interrupts vs. hand allocation vs. PCI routing, for example). It is much 
easier to do this correctly at bus attach time when the resource lists 
are made (how PPC does it).


(1) is easy to fix without API changes, but (2) and (3) are fundamental 
architectural problems that will bite us immediately down the road and 
cause a permanent schism between OF support on different platforms.


Let me describe how this is handled on PowerPC (Linux on PPC solves the 
problem the same way). When constructing a resource list, bus drivers 
that construct them from OF properties call ofw_bus_map_intr() with the 

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Michal Meloun]

Dne 19.07.2016 v 2:11 Nathan Whitehorn napsal(a):
Hi Nathan,
I’m afraid that skra is on vacation, for next 2 weeks (at minimum), so
please don’t expect quick response.

> Could you please describe what this change is in more detail?
Short description is appended.

> 
> It breaks a lot of encapsulations we have worked very hard to maintain,
> moves ARM code into MI parts of the kernel, and the OFW parts violate
> IEEE 1275 (the Open Firmware standard). In particular, there is no
> guarantee that the interrupts for a newbus (or OF) device are encoded in
> a property called "interrupts" (or, indeed, in any property at all) on
> that node and there are many, many device trees where that is not the
> case (e.g. ones with interrupt maps, as well as Apple hardware). By
> putting that knowledge into the OF root bus device, which we have tried
> to keep it out of, this enforces a standard that doesn't actually exist.
Imho, this patch doesn’t change anything in this area. Only handling of
“interrupts” property is changed, all other cases are unchanged (I
hope).  Also, INTRNG code is currently shared by ARM, ARM64 and MIPS.

> 
> I'm hesitant to ask for reversion on something that landed 6 weeks ago
> without me noticing, but this needs a lot more architectural work before
> any parts of the kernel should use it.
> -Nathan
I think that it’s too late.  This patch series consist of r301451
(https://reviews.freebsd.org/D6632),
r301453, r301539 and 301543.  And new GPIO interrupts are currently used
(by in tree drivers or in development trees).



The root of problem is that standard way of delivering interrupt
resource to consumer driver doesn’t works in OFW world.

So we have some fact:
- the format of interrupt property is dependent of interrupt
  controller and only interrupt controller can parse it.
- the interrupt property can have more data than just interrupt
  number.
- single interrupt controller must be able to handle multiple
  format of interrupt description.

In pre-patchset era, simplebus enumerates children and attempts to set
memory and interrupts to resource list for them. But the interrupt
controllers are not yet populated so nobody can parse interrupt
property. Moreover, in all cases (parsed or not), we cannot store
complete interrupt description into resource list.

The patch simply postpones reading of interrupt property to
bus_alloc_resource() (called by consumer driver) time.

Due to this, we can:
- parse  interrupt property. The interrupt driver must exist
  at this time.
- bus_alloc_resource() returns resource, so we can attach parsed
  interrupt data to it. By this, the resource itself can be used
  for delivering configuration data to subsequent call to
  bus_setup_intr() (or to all related  bus_() calls).


The patched code still accepts delivering of interrupts in resource list.

Michal

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Nathan Whitehorn]

Could you please describe what this change is in more detail?

It breaks a lot of encapsulations we have worked very hard to maintain, 
moves ARM code into MI parts of the kernel, and the OFW parts violate 
IEEE 1275 (the Open Firmware standard). In particular, there is no 
guarantee that the interrupts for a newbus (or OF) device are encoded in 
a property called "interrupts" (or, indeed, in any property at all) on 
that node and there are many, many device trees where that is not the 
case (e.g. ones with interrupt maps, as well as Apple hardware). By 
putting that knowledge into the OF root bus device, which we have tried 
to keep it out of, this enforces a standard that doesn't actually exist.


I'm hesitant to ask for reversion on something that landed 6 weeks ago 
without me noticing, but this needs a lot more architectural work before 
any parts of the kernel should use it.

-Nathan

On 06/05/16 09:20, Svatopluk Kraus wrote:

Author: skra
Date: Sun Jun  5 16:20:12 2016
New Revision: 301453
URL: https://svnweb.freebsd.org/changeset/base/301453

Log:
   INTRNG - change the way how an interrupt mapping data are provided
   to the framework in OFW (FDT) case.
   
   This is a follow-up to r301451.
   
   Differential Revision:	https://reviews.freebsd.org/D6634


Modified:
   head/sys/arm/arm/nexus.c
   head/sys/arm64/arm64/gic_v3.c
   head/sys/arm64/arm64/nexus.c
   head/sys/dev/fdt/simplebus.c
   head/sys/dev/gpio/ofw_gpiobus.c
   head/sys/dev/iicbus/ofw_iicbus.c
   head/sys/dev/ofw/ofw_bus_subr.c
   head/sys/dev/ofw/ofw_bus_subr.h
   head/sys/dev/ofw/ofwbus.c
   head/sys/dev/pci/pci_host_generic.c
   head/sys/dev/vnic/mrml_bridge.c
   head/sys/dev/vnic/thunder_mdio_fdt.c
   head/sys/kern/subr_intr.c
   head/sys/mips/mips/nexus.c
   head/sys/sys/intr.h

Modified: head/sys/arm/arm/nexus.c
==
--- head/sys/arm/arm/nexus.cSun Jun  5 16:09:31 2016(r301452)
+++ head/sys/arm/arm/nexus.cSun Jun  5 16:20:12 2016(r301453)
@@ -412,6 +412,10 @@ nexus_ofw_map_intr(device_t dev, device_
  pcell_t *intr)
  {
  
+#ifdef INTRNG

+   return (INTR_IRQ_INVALID);
+#else
return (intr_fdt_map_irq(iparent, intr, icells));
+#endif
  }
  #endif

Modified: head/sys/arm64/arm64/gic_v3.c
==
--- head/sys/arm64/arm64/gic_v3.c   Sun Jun  5 16:09:31 2016
(r301452)
+++ head/sys/arm64/arm64/gic_v3.c   Sun Jun  5 16:20:12 2016
(r301453)
@@ -58,6 +58,10 @@ __FBSDID("$FreeBSD$");
  #include 
  #include 
  
+#ifdef FDT

+#include 
+#endif
+
  #include "pic_if.h"
  
  #include "gic_v3_reg.h"


Modified: head/sys/arm64/arm64/nexus.c
==
--- head/sys/arm64/arm64/nexus.cSun Jun  5 16:09:31 2016
(r301452)
+++ head/sys/arm64/arm64/nexus.cSun Jun  5 16:20:12 2016
(r301453)
@@ -448,7 +448,7 @@ nexus_ofw_map_intr(device_t dev, device_
  pcell_t *intr)
  {
  #ifdef INTRNG
-   return (intr_fdt_map_irq(iparent, intr, icells));
+   return (INTR_IRQ_INVALID);
  #else
int irq;
  


Modified: head/sys/dev/fdt/simplebus.c
==
--- head/sys/dev/fdt/simplebus.cSun Jun  5 16:09:31 2016
(r301452)
+++ head/sys/dev/fdt/simplebus.cSun Jun  5 16:20:12 2016
(r301453)
@@ -251,7 +251,9 @@ simplebus_setup_dinfo(device_t dev, phan
  
  	resource_list_init(>rl);

ofw_bus_reg_to_rl(dev, node, sc->acells, sc->scells, >rl);
+#ifndef INTRNG
ofw_bus_intr_to_rl(dev, node, >rl, NULL);
+#endif
  
  	return (ndi);

  }

Modified: head/sys/dev/gpio/ofw_gpiobus.c
==
--- head/sys/dev/gpio/ofw_gpiobus.c Sun Jun  5 16:09:31 2016
(r301452)
+++ head/sys/dev/gpio/ofw_gpiobus.c Sun Jun  5 16:20:12 2016
(r301453)
@@ -321,11 +321,13 @@ ofw_gpiobus_setup_devinfo(device_t bus,
devi->pins[i] = pins[i].pin;
}
free(pins, M_DEVBUF);
+#ifndef INTRNG
/* Parse the interrupt resources. */
if (ofw_bus_intr_to_rl(bus, node, >opd_dinfo.rl, NULL) != 0) {
ofw_gpiobus_destroy_devinfo(bus, dinfo);
return (NULL);
}
+#endif
device_set_ivars(child, dinfo);
  
  	return (dinfo);


Modified: head/sys/dev/iicbus/ofw_iicbus.c
==
--- head/sys/dev/iicbus/ofw_iicbus.cSun Jun  5 16:09:31 2016
(r301452)
+++ head/sys/dev/iicbus/ofw_iicbus.cSun Jun  5 16:20:12 2016
(r301453)
@@ -187,8 +187,10 @@ ofw_iicbus_attach(device_t dev)
  
  		childdev = device_add_child(dev, NULL, -1);

resource_list_init(>opd_dinfo.rl);
+#ifndef INTRNG

svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys

[Svatopluk Kraus]

Author: skra
Date: Sun Jun  5 16:20:12 2016
New Revision: 301453
URL: https://svnweb.freebsd.org/changeset/base/301453

Log:
  INTRNG - change the way how an interrupt mapping data are provided
  to the framework in OFW (FDT) case.
  
  This is a follow-up to r301451.
  
  Differential Revision:https://reviews.freebsd.org/D6634

Modified:
  head/sys/arm/arm/nexus.c
  head/sys/arm64/arm64/gic_v3.c
  head/sys/arm64/arm64/nexus.c
  head/sys/dev/fdt/simplebus.c
  head/sys/dev/gpio/ofw_gpiobus.c
  head/sys/dev/iicbus/ofw_iicbus.c
  head/sys/dev/ofw/ofw_bus_subr.c
  head/sys/dev/ofw/ofw_bus_subr.h
  head/sys/dev/ofw/ofwbus.c
  head/sys/dev/pci/pci_host_generic.c
  head/sys/dev/vnic/mrml_bridge.c
  head/sys/dev/vnic/thunder_mdio_fdt.c
  head/sys/kern/subr_intr.c
  head/sys/mips/mips/nexus.c
  head/sys/sys/intr.h

Modified: head/sys/arm/arm/nexus.c
==
--- head/sys/arm/arm/nexus.cSun Jun  5 16:09:31 2016(r301452)
+++ head/sys/arm/arm/nexus.cSun Jun  5 16:20:12 2016(r301453)
@@ -412,6 +412,10 @@ nexus_ofw_map_intr(device_t dev, device_
 pcell_t *intr)
 {
 
+#ifdef INTRNG
+   return (INTR_IRQ_INVALID);
+#else
return (intr_fdt_map_irq(iparent, intr, icells));
+#endif
 }
 #endif

Modified: head/sys/arm64/arm64/gic_v3.c
==
--- head/sys/arm64/arm64/gic_v3.c   Sun Jun  5 16:09:31 2016
(r301452)
+++ head/sys/arm64/arm64/gic_v3.c   Sun Jun  5 16:20:12 2016
(r301453)
@@ -58,6 +58,10 @@ __FBSDID("$FreeBSD$");
 #include 
 #include 
 
+#ifdef FDT
+#include 
+#endif
+
 #include "pic_if.h"
 
 #include "gic_v3_reg.h"

Modified: head/sys/arm64/arm64/nexus.c
==
--- head/sys/arm64/arm64/nexus.cSun Jun  5 16:09:31 2016
(r301452)
+++ head/sys/arm64/arm64/nexus.cSun Jun  5 16:20:12 2016
(r301453)
@@ -448,7 +448,7 @@ nexus_ofw_map_intr(device_t dev, device_
 pcell_t *intr)
 {
 #ifdef INTRNG
-   return (intr_fdt_map_irq(iparent, intr, icells));
+   return (INTR_IRQ_INVALID);
 #else
int irq;
 

Modified: head/sys/dev/fdt/simplebus.c
==
--- head/sys/dev/fdt/simplebus.cSun Jun  5 16:09:31 2016
(r301452)
+++ head/sys/dev/fdt/simplebus.cSun Jun  5 16:20:12 2016
(r301453)
@@ -251,7 +251,9 @@ simplebus_setup_dinfo(device_t dev, phan
 
resource_list_init(>rl);
ofw_bus_reg_to_rl(dev, node, sc->acells, sc->scells, >rl);
+#ifndef INTRNG
ofw_bus_intr_to_rl(dev, node, >rl, NULL);
+#endif
 
return (ndi);
 }

Modified: head/sys/dev/gpio/ofw_gpiobus.c
==
--- head/sys/dev/gpio/ofw_gpiobus.c Sun Jun  5 16:09:31 2016
(r301452)
+++ head/sys/dev/gpio/ofw_gpiobus.c Sun Jun  5 16:20:12 2016
(r301453)
@@ -321,11 +321,13 @@ ofw_gpiobus_setup_devinfo(device_t bus, 
devi->pins[i] = pins[i].pin;
}
free(pins, M_DEVBUF);
+#ifndef INTRNG
/* Parse the interrupt resources. */
if (ofw_bus_intr_to_rl(bus, node, >opd_dinfo.rl, NULL) != 0) {
ofw_gpiobus_destroy_devinfo(bus, dinfo);
return (NULL);
}
+#endif
device_set_ivars(child, dinfo);
 
return (dinfo);

Modified: head/sys/dev/iicbus/ofw_iicbus.c
==
--- head/sys/dev/iicbus/ofw_iicbus.cSun Jun  5 16:09:31 2016
(r301452)
+++ head/sys/dev/iicbus/ofw_iicbus.cSun Jun  5 16:20:12 2016
(r301453)
@@ -187,8 +187,10 @@ ofw_iicbus_attach(device_t dev)
 
childdev = device_add_child(dev, NULL, -1);
resource_list_init(>opd_dinfo.rl);
+#ifndef INTRNG
ofw_bus_intr_to_rl(childdev, child,
>opd_dinfo.rl, NULL);
+#endif
device_set_ivars(childdev, dinfo);
}
 

Modified: head/sys/dev/ofw/ofw_bus_subr.c
==
--- head/sys/dev/ofw/ofw_bus_subr.c Sun Jun  5 16:09:31 2016
(r301452)
+++ head/sys/dev/ofw/ofw_bus_subr.c Sun Jun  5 16:20:12 2016
(r301453)
@@ -516,6 +516,7 @@ ofw_bus_find_iparent(phandle_t node)
return (iparent);
 }
 
+#ifndef INTRNG
 int
 ofw_bus_intr_to_rl(device_t dev, phandle_t node,
 struct resource_list *rl, int *rlen)
@@ -581,6 +582,78 @@ ofw_bus_intr_to_rl(device_t dev, phandle
free(intr, M_OFWPROP);
return (err);
 }
+#endif
+
+int
+ofw_bus_intr_by_rid(device_t dev, phandle_t node, int wanted_rid,
+phandle_t *producer, int *ncells, pcell_t **cells)
+{
+   phandle_t iparent;
+   uint32_t icells, *intr;
+