From owner-svn-src-all@freebsd.org Thu Dec 17 17:22:24 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id B8815A49624; Thu, 17 Dec 2015 17:22:24 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from d.mail.sonic.net (d.mail.sonic.net [64.142.111.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 9B76F17B0; Thu, 17 Dec 2015 17:22:24 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from zeppelin.tachypleus.net ([32.210.31.157]) (authenticated bits=0) by d.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id tBHHBt07008109 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Thu, 17 Dec 2015 09:11:56 -0800 Subject: Re: svn commit: r292405 - in head/sys: arm64/include dev/ofw To: Andrew Turner , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201512171700.tBHH04QC021609@repo.freebsd.org> From: Nathan Whitehorn Message-ID: <5672ECDA.80101@freebsd.org> Date: Thu, 17 Dec 2015 09:11:54 -0800 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <201512171700.tBHH04QC021609@repo.freebsd.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Sonic-CAuth: UmFuZG9tSVbH24y7siwTj00xzbJDRIcL1evfCIT9d/mnqEciavwzHsWwsY/M74T0CUNO8OtRQlguksOSnNrxA4GwwjBC49hvOscOjLFy9e4= X-Sonic-ID: C;AEbPReGk5RGXocgxU3XIUw== M;wp1kRuGk5RGXocgxU3XIUw== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Dec 2015 17:22:24 -0000 I don't really like the #ifdef here. If it contradicts ePAPR, switching on the behavior through a #define OFW_EPAPR is at best misleading. And, aside from the default value of 2, this *is* compliant with the ePAPR spec, as well as with CHRP and PAPR (which is where ePAPR got it from). Do we have any in-the-wild CHRP-ish device trees for which #address-cells is unspecified to even worry about the default anyway? In any event, I'd much prefer that it be explicitly disabled on SPARC, which is the odd man out on our platforms, rather than opt-in on all other platforms, which use CHRP-heritage bindings, and that the #define be something more descriptive/correct like #define OFW_IMAP_NO_INTERRUPT_PARENT. -Nathan On 12/17/15 09:00, Andrew Turner wrote: > Author: andrew > Date: Thu Dec 17 17:00:04 2015 > New Revision: 292405 > URL: https://svnweb.freebsd.org/changeset/base/292405 > > Log: > Support the variant of the interrupt-map property where the parent bus has > the #address-cells property set. For this we need to read more data before > the parent interrupt description. > > this is only enabled on arm64 for now as it's not quite compliant with the > ePAPR spec. We should use a default of 2 where the #address-cells property > is missing, however this will need further testing across architectures. > > Obtained from: ABT Systems Ltd > Sponsored by: SoftIron Inc > Differential Revision: https://reviews.freebsd.org/D4518 > > Modified: > head/sys/arm64/include/ofw_machdep.h > head/sys/dev/ofw/ofw_bus_subr.c > > Modified: head/sys/arm64/include/ofw_machdep.h > ============================================================================== > --- head/sys/arm64/include/ofw_machdep.h Thu Dec 17 16:09:15 2015 (r292404) > +++ head/sys/arm64/include/ofw_machdep.h Thu Dec 17 17:00:04 2015 (r292405) > @@ -41,4 +41,7 @@ struct mem_region { > vm_size_t mr_size; > }; > > +/* FDT follows ePAPR */ > +#define OFW_EPAPR > + > #endif /* _MACHINE_OFW_MACHDEP_H_ */ > > Modified: head/sys/dev/ofw/ofw_bus_subr.c > ============================================================================== > --- head/sys/dev/ofw/ofw_bus_subr.c Thu Dec 17 16:09:15 2015 (r292404) > +++ head/sys/dev/ofw/ofw_bus_subr.c Thu Dec 17 17:00:04 2015 (r292405) > @@ -341,6 +341,7 @@ ofw_bus_search_intrmap(void *intr, int i > uint8_t *uiregs = regs; > uint8_t *uiimapmsk = imapmsk; > uint8_t *mptr; > + pcell_t paddrsz; > pcell_t pintrsz; > int i, rsz, tsz; > > @@ -357,19 +358,31 @@ ofw_bus_search_intrmap(void *intr, int i > > mptr = imap; > i = imapsz; > + paddrsz = 0; > while (i > 0) { > bcopy(mptr + physsz + intrsz, &parent, sizeof(parent)); > +#ifdef OFW_EPAPR > + /* > + * Find if we need to read the parent address data. Sparc64 > + * uses a different encoding that doesn't include this data. > + */ > + if (OF_getencprop(OF_node_from_xref(parent), > + "#address-cells", &paddrsz, sizeof(paddrsz)) == -1) > + paddrsz = 0; /* default */ > + paddrsz *= sizeof(pcell_t); > +#endif > + > if (OF_searchencprop(OF_node_from_xref(parent), > "#interrupt-cells", &pintrsz, sizeof(pintrsz)) == -1) > pintrsz = 1; /* default */ > pintrsz *= sizeof(pcell_t); > > /* Compute the map stride size. */ > - tsz = physsz + intrsz + sizeof(phandle_t) + pintrsz; > + tsz = physsz + intrsz + sizeof(phandle_t) + paddrsz + pintrsz; > KASSERT(i >= tsz, ("ofw_bus_search_intrmap: truncated map")); > > if (bcmp(ref, mptr, physsz + intrsz) == 0) { > - bcopy(mptr + physsz + intrsz + sizeof(parent), > + bcopy(mptr + physsz + intrsz + sizeof(parent) + paddrsz, > result, MIN(rintrsz, pintrsz)); > > if (iparent != NULL) >