From owner-svn-src-all@freebsd.org Wed Feb 8 12:58:43 2017 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 0C7D8CD6E54; Wed, 8 Feb 2017 12:58:43 +0000 (UTC) (envelope-from onwahe@gmail.com) Received: from mail-io0-x241.google.com (mail-io0-x241.google.com [IPv6:2607:f8b0:4001:c06::241]) (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 C8D6F1FA5; Wed, 8 Feb 2017 12:58:42 +0000 (UTC) (envelope-from onwahe@gmail.com) Received: by mail-io0-x241.google.com with SMTP id m98so15744440iod.2; Wed, 08 Feb 2017 04:58:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=Y0Ovd0dS+Wgo2zamPXH2Sa00VqLWVPSbx6SJzm6gwws=; b=QDzBW0pYW8t/B+LoreSgAL+q7w9qnxUJ62Dsjp//azTW49cBtZ28BeE96qAIR3Ow2L /jPmMHlkj7jhjty2DaladksNgLdtsxaataqv74u5MRATbZmKtw5+CuMlzNBOXn3PXmr3 7/LgwOkYZVPTDEaAWhvMgYkG6FC4nZkbZKiVh8Ov+NbBlsnxnmmHbzFgXZHR2TUq5vtz /AjYSEB3Wpd7FelESrXSg/EaeqByuxL3h5AMKmZ2CrBoDCb6pYDMRH6lUql/H0RC4AzZ Bbg5YElcC/n7FdObPJatild7TBdFq2JBDMdhDa2iTwx/RVCTQYhhYQ8WU6QjOf4CsTc/ IeVw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Y0Ovd0dS+Wgo2zamPXH2Sa00VqLWVPSbx6SJzm6gwws=; b=bmSxhpMKZsm4xFnJzFPz7+iRCcLDdziTodni6AufGtSvCbB8Pn068CxfyDZtReKOgX bwDmJNlqpkL+Kih43b/TzaFUT7d+YuE/PVzGmB4rnGf5sKyBdCtZGHkZY+Qyi9SUDh+t 9SUm8tJiilqRYFEegX3BA5rcgbgzsNrd4NCtbfslgCfUQ7f42yaFKbK+izLR0+tf2nEX 5T4lG1o0zNdR4otVDynWWz0nPknFRtgl7Q7UCoD6YF1jhzqEzKLKIN5ifd9XsCNJQ9CS gr9MZ3P+uPop2GtYzCz+1p+cumKb+LUlaZdZtcHDo5bfzqzOaPCR2YXxA9yWJaPdrA0W S9cQ== X-Gm-Message-State: AMke39kYTzapnWHRn8e2A2izxeIrhFhDPrytxFkext8hhEW9/Cu1SBGbXS7hdDYHHyf3HoIiunUcxNEvhGoEhQ== X-Received: by 10.107.152.144 with SMTP id a138mr9935068ioe.207.1486558722107; Wed, 08 Feb 2017 04:58:42 -0800 (PST) MIME-Version: 1.0 Received: by 10.64.101.194 with HTTP; Wed, 8 Feb 2017 04:58:41 -0800 (PST) In-Reply-To: <20170207113844.2cd08852@zapp> References: <201702061308.v16D8nGC071178@repo.freebsd.org> <20170207113844.2cd08852@zapp> From: Svatopluk Kraus Date: Wed, 8 Feb 2017 13:58:41 +0100 Message-ID: Subject: Re: svn commit: r313339 - head/sys/kern To: Andrew Turner Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset=UTF-8 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 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: Wed, 08 Feb 2017 12:58:43 -0000 On Tue, Feb 7, 2017 at 12:38 PM, Andrew Turner wrote: > On Tue, 7 Feb 2017 11:35:48 +0100 > Svatopluk Kraus wrote: >> Does an xref refer only to one hardware? Or two hardwares may have >> same xrefs? Why one hardware cannot be represented by one PIC in >> INTRNG even if two drivers exist for it? > > The xref is an FDT thing, in INTRNG we use it as a handle to hardware. > This changes it so each of these handles refers to hardware within one > of two spaces, the PIC space and MSI space. It may be that for FDT > these spaces are non overlapping, however with ACPI it will simplify > the code to allow us to have the same handle refer to different > controllers in different spaces. In INTRNG, the xref is not FDT thing, but general hardware identifier which is intented to be unique in a system. See intr_map_irq(device_t dev, intptr_t xref, struct intr_map_data *data). The xref should be independent on mapping data. INTRNG should know nothing about mapping data, but your change just changed that. The xref is opaque for INTRNG too. So, I do not see where is the problem to have unique xref identifiers in a system. The xref can be defined that way. So, do you have a problem with overlapping spaces or you just want to have more PICs for one xref? > >> On Mon, Feb 6, 2017 at 2:08 PM, Andrew Turner >> wrote: >> > Author: andrew >> > Date: Mon Feb 6 13:08:48 2017 >> > New Revision: 313339 >> > URL: https://svnweb.freebsd.org/changeset/base/313339 >> > >> > Log: >> > Only allow the pic type to be either a PIC or MSI type. All >> > interrupt controller drivers handle either MSI/MSI-X interrupts, or >> > regular interrupts, as such enforce this in the interrupt handling >> > framework. If a later driver was to handle both it would need to >> > create one of each. >> > >> > This will allow future changes to allow the xref space to >> > overlap, but refer to different drivers. >> > >> > Obtained from: ABT Systems Ltd >> > Sponsored by: The FreeBSD Foundation >> > X-Differential Revision: https://reviews.freebsd.org/D8616 > ... >> > @@ -822,13 +827,13 @@ intr_pic_claim_root(device_t dev, intptr >> > { >> > struct intr_pic *pic; >> > >> > - pic = pic_lookup(dev, xref); >> > + pic = pic_lookup(dev, xref, FLAG_PIC); >> > if (pic == NULL) { >> > device_printf(dev, "not registered\n"); >> > return (EINVAL); >> > } >> > >> > - KASSERT((pic->pic_flags & FLAG_PIC) != 0, >> > + KASSERT((pic->pic_flags & FLAG_TYPE_MASK) == FLAG_PIC, >> > ("%s: Found a non-PIC controller: %s", __func__, >> > device_get_name(pic->pic_dev))); >> > >> >> So, you would check that pic_lookup(dev, xref, FLAG_PIC) returns PIC >> with FLAG_PIC set? >> Really? This nonsense is in other places too. If you would like to be >> this way, check it only in pic_lookup() then! > > It's there so if someone makes a change to pic_lookup that breaks > this assumption they will be told about it. For me, it's too much. It's like calling mtx_lock() and immediately check that the mutex is locked. It's mental to check that a function really returned what was requested. Especially, when the function returns NULL in case that the request cannot be met. Should anyone check any function that it does what is announced?