Re: Testing an Open Firmware interrupt-map patch

On Tue, Dec 15, 2015 at 12:28:30AM +0000, Andrew Turner wrote:
> On Tue, 15 Dec 2015 00:47:10 +0100
> Marius Strobl <[email protected]> wrote:
> 
> > On Mon, Dec 14, 2015 at 01:46:25PM +0000, Andrew Turner wrote:
> > > Hello,
> > > 
> > > I'm looking for testers for a patch to update how we parse the
> > > interrupt-map property to follow the ePAPR spec. This property is
> > > commonly used with PCIe controllers.
> > > 
> > > The current code doesn't take the parent address size property. When
> > > this is non-zero we need to also read these values. This is needed
> > > on an arm64 board I have as the interrupt parent has memory-mapped
> > > children so needs this to be set.
> > > 
> > > I have a patch at [1] to add this, however would like it if this
> > > could be tested on other platforms using this code to check it
> > > doesn't break these platforms before I commit it.  
> > 
> > +		    "#address-cells", &paddrsz, sizeof(paddrsz)) ==
> > -1)
> > +			paddrsz = 0;	/* default */
> > 
> > According to IEEE 1275-1994 (page 110) the default in case of a
> > "#address-cells" property is 2 (see also ofw_bus_setup_iinfo()),
> > with the latter also being known as the correct default value
> > for sparc64 machines.
> 
> However both Linux and NetBSD have similar code, both assume a missing
> interrupt-parent #address-size property the correct value is 0.

According to ePAPR v1.1, the default in case of missing "#address-cells"
properties also is 2 (page 24). However, it also says that the open-pic
node/parent interrupt domain has an "#address-cells" value of 0 (page 36).
So the above snippet appears more like a workaround for broken device
trees as far as ePAPR is concerned, at least the comment "default" is
misleading in that regard.

> > In any case, this will have quite an impact on the offset
> > calculation on sparc64 machines for e. g. EBusses beneath PCI
> > busses (which use 3 address cells there) and, thus, most likely
> > will cause havoc. Maybe the parent unit address should be only
> > taken into account on platforms using interrupt parents?
> 
> It would seem there is a difference between the encoding on Sparc64 and
> ePAPR. Using the the parent #address-size may lead to incorrect parsing
> on Sparc64, as such I will disable it there.
> 

Yes, as the name suggests, ePAPR is a specification for the Power
Architecture. Originally, ofw_bus_search_intrmap() also was sparc64
MD code until nwhitehorn@ in r186128 brought it over to the truly MI
sys/dev/ofw/ofw_bus_subr.c I had started.
Actually, as for the parent unit address in the interrupt map table
ePAPR v1.1 specifically refers to the interrupt parent (page 34).
So in order to avoid an #if !defined(__sparc64__) in MI code it seems
best to do what I suggested above and to apply paddrsz only on the
platforms that actually use interrupt parents. Alternatively, please
at least invert it to an #if defined(OFW_EPAPR) and define OFW_EPAPR
in sys/<arch>/include/ofw_machdep.h as applicable.

Marius

_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-ppc
To unsubscribe, send any mail to "[email protected]"