From owner-freebsd-arm@freebsd.org Fri Jul 29 23:16:27 2016 Return-Path: Delivered-To: freebsd-arm@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 332D8BA75D6; Fri, 29 Jul 2016 23:16:27 +0000 (UTC) (envelope-from onwahe@gmail.com) Received: from mail-io0-f179.google.com (mail-io0-f179.google.com [209.85.223.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id F24F211EA; Fri, 29 Jul 2016 23:16:26 +0000 (UTC) (envelope-from onwahe@gmail.com) Received: by mail-io0-f179.google.com with SMTP id q83so142358604iod.1; Fri, 29 Jul 2016 16:16:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=1/dzjY4rjQhu54eGdT3PiIKE5UVeyEAndD2vRgbSnzI=; b=aETbDCHCz4ywEilPcMPYCXkMwL9Tc6Wu0e8QeMDoxL35I4iznmFyihDvLmMcgvgneZ +Ew3snOLOyLq+xRagq1B2zHDCwJxWEspsrAVfCE/lSmt0WxExa/wOcySCvUYphY7f7UL /gq6chGap5kZscmOdZ6qhQTODfCmhU0sz/XgEn5AW+vsJoemjekoNIFB6tObUfohWLvQ LrH/q5aIMyZ6au6II3cY5BuClKEAFP12I8LqF5ZYLXVKAesewUC1e6gUSDKtKn6XKVh3 v26SSuxa+DYpw5GhJzEYsigiKxEM7ET4Ugv0yAPqukoNzp3ogKP5jrTi3gNJ+M7g6dPb g8GQ== X-Gm-Message-State: AEkoouv4rjU94FXcr2P2NwpXt0xA9ghnpZffjVLy7F6qeygb5vv6Mequ0oeqb4WpP+v7Hw== X-Received: by 10.107.175.27 with SMTP id y27mr52111077ioe.137.1469830590332; Fri, 29 Jul 2016 15:16:30 -0700 (PDT) Received: from mail-io0-f177.google.com (mail-io0-f177.google.com. [209.85.223.177]) by smtp.gmail.com with ESMTPSA id e201sm2152444itc.21.2016.07.29.15.16.30 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 29 Jul 2016 15:16:30 -0700 (PDT) Received: by mail-io0-f177.google.com with SMTP id b62so140912024iod.3; Fri, 29 Jul 2016 15:16:30 -0700 (PDT) X-Received: by 10.107.142.129 with SMTP id q123mr38715103iod.84.1469830589842; Fri, 29 Jul 2016 15:16:29 -0700 (PDT) MIME-Version: 1.0 Received: by 10.64.162.75 with HTTP; Fri, 29 Jul 2016 15:16:28 -0700 (PDT) In-Reply-To: References: <201606051620.u55GKD5S066398@repo.freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> <5798E104.5020104@FreeBSD.org> <579A25BB.8070206@FreeBSD.org> <30790e40-58b4-3371-c0f0-b7545571f389@freebsd.org> <579AFFC5.1040005@FreeBSD.org> From: Svatopluk Kraus Date: Sat, 30 Jul 2016 00:16:28 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: INTRNG (Was: svn commit: r301453....) To: Nathan Whitehorn Cc: Michal Meloun , "freebsd-arm@freebsd.org" , "freebsd-arch@freebsd.org" Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "Porting FreeBSD to ARM processors." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jul 2016 23:16:27 -0000 Well, I'm online again, unfortanately, I'm quite busy, so just quick response for now to the formal ask for the reversion. On Fri, Jul 29, 2016 at 4:41 PM, Nathan Whitehorn wrote: > [snip] > > So here is where I am right now: > - The perceived advantage of the new API seems to be that the mapping > information (interrupt parent, etc.) ends up in a struct resource instead of > in a centralized mapping table It's more than that. The change has made INTRNG framework not dependent on OFW(FDT) stuff. So after next r301543, the framework is clean of all additions related to various buses. It was something I wanted to do before FreeBSD-11 release, once when adrian@ moved INTRNG to sys/kern. The framework is used by arm, arm64 and mips now, so from my point of view, it was quite important. The way how it was done is not perfect and may be changed in the future. However, it does not break anything, does not change old functionality, and only few bus drivers were modified slightly. It was also only way which I and Michal have agreed on considering current kernel code. > - Additionally, it gives you a better shot at having the PIC online before > the PIC's interrupts are parsed (which is not required, but nice). For me, it's just correct design. Please, not all world is about OFW. > - Beyond these aesthetic points, there are no concrete examples of new > functionality added by this API, aside from some minor implementation bugs > of the old one on ARM that are easily fixed. Please, don't use subjective attributes like "aesthetic". > - There are, conversely, a number of concrete cases where this new API would > not be able to do the right thing. Some of these can be worked around or > fixed with significant restructuring in the MI parts of the kernel. I have not looked yet at these concrete cases, but which MI parts of kernel do you think of? It may be supposed that some bus drivers would be modified, but not MI parts of kernel! > - If we have both, the interrupt handling code in the MI parts of the kernel > will bifurcate. This patch alone has added a bunch of #ifdef to > kern/subr_bus.c, a new file to kern/, and lots of #ifdef to dev/ofw. This is > going to be really hard to maintain if we need both. IMO, this point is bogus totally. The #ifdefs in kern/subr_bus.c were added just to be polite to not-INTRNG kernels. They can be removed. No one new file was added. Lots of #ifdef in dev/ofw were added because ofw_bus_map_intr() return value is not checked in ofw_bus_intr_to_rl(), so resource list entry is always added. They are there also to clearly manifest INTRNG needs. > > Is this all correct? > I don't think so. Further, considering the reversion, I don't think that it can be done simply now. I suppose that more files were changed afterwards when no bus header is polluted by the framework now. However, as I have already wrote, this part of INTRNG may be changed to serve well for everyone. On the other hand, IMO, a centralized global interrupt mapping table is not good design, and if established after all, it should not be a part of the framework. That said, I suggest to continue with work on INTRNG. Svata > If so, can we please back this out until this discussion is complete? I'm > asking this formally at this point, under the Committer's Guide section > about reversion requests. > -Nathan > >