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