From owner-svn-src-head@FreeBSD.ORG Sun May 10 02:35:35 2015 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 0B50F64E; Sun, 10 May 2015 02:35:35 +0000 (UTC) Received: from mail-la0-x22f.google.com (mail-la0-x22f.google.com [IPv6:2a00:1450:4010:c03::22f]) (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 8CADE10AA; Sun, 10 May 2015 02:35:34 +0000 (UTC) Received: by laat2 with SMTP id t2so73935376laa.1; Sat, 09 May 2015 19:35:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=caj+g9oEO0O3NO5a24IIJk9kl8pjXgZUZtnEiim9qTk=; b=emNTpQXoI5fh+OxBrqsmd+Kz0sRa9TPE1Kfo+v7Qo/nx5ERnf2YbE/zSIikCnR1c1Y JjXy5NHNuH88yH58Qd9GtISb6fRZRqg+t7Cl8MJw0/xLTSIc3mJ/MBiiHJM6foGNJTcl fbXl/tMnl6FJeSmveRo3z2DqPPF+y26gUigRFSheWeDpGv7pYK5mPvlsZFzR3RSp7LTw qdJwSOBZrarl+zA+RpzBVblwyrtCbrHsTPAjyV+y0zM6e9XAhkX9mh4evtdVdERJqzzh pKFPGkk3jxCbhnAXhFzhUct9HVZ+E63rRSZEaxNSXFS+9kYn1Ho+p/5EAFPwNF7hDDYa 72jw== MIME-Version: 1.0 X-Received: by 10.152.29.67 with SMTP id i3mr3532900lah.64.1431225332545; Sat, 09 May 2015 19:35:32 -0700 (PDT) Received: by 10.112.154.166 with HTTP; Sat, 9 May 2015 19:35:32 -0700 (PDT) In-Reply-To: <2165864.Vg8k3tllF7@ralph.baldwin.cx> References: <201505090305.t4935jYk086983@svn.freebsd.org> <2165864.Vg8k3tllF7@ralph.baldwin.cx> Date: Sat, 9 May 2015 23:35:32 -0300 Message-ID: Subject: Re: svn commit: r282674 - in head/sys/dev: iicbus ofw From: Luiz Otavio O Souza To: John Baldwin Cc: Luiz Otavio O Souza , 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-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 10 May 2015 02:35:35 -0000 On Sat, May 9, 2015 at 7:45 AM, John Baldwin wrote: > On Saturday, May 09, 2015 03:05:45 AM Luiz Otavio O Souza wrote: >> Author: loos >> Date: Sat May 9 03:05:44 2015 >> New Revision: 282674 >> URL: https://svnweb.freebsd.org/changeset/base/282674 >> >> Log: >> Handle IRQ resources on iicbus and ofw_iicbus. >> >> Based on a patch submitted by Michal Meloun . >> >> Modified: >> head/sys/dev/iicbus/iicbus.c >> head/sys/dev/iicbus/iicbus.h >> head/sys/dev/ofw/ofw_iicbus.c >> >> Modified: head/sys/dev/iicbus/iicbus.c >> ============================================================================== >> --- head/sys/dev/iicbus/iicbus.c Sat May 9 00:48:44 2015 (r282673) >> +++ head/sys/dev/iicbus/iicbus.c Sat May 9 03:05:44 2015 (r282674) >> @@ -157,9 +159,9 @@ iicbus_probe_nomatch(device_t bus, devic >> { >> struct iicbus_ivar *devi = IICBUS_IVAR(child); >> >> - device_printf(bus, ""); >> - printf(" at addr %#x\n", devi->addr); >> - return; >> + device_printf(bus, " at addr %#x", devi->addr); >> + resource_list_print_type(&devi->rl, "irq", SYS_RES_IRQ, "%ld"); >> + printf("\n"); > > Other bus drivers do not print resources for nomatch devices (or at > least PCI doesn't). Given that, I'm not sure it makes sense to do > that here? That's right, it was an overzealous from my part. > >> +static int >> +iicbus_set_resource(device_t dev, device_t child, int type, int rid, >> + u_long start, u_long count) >> +{ >> + struct iicbus_ivar *devi; >> + struct resource_list_entry *rle; >> + >> + devi = IICBUS_IVAR(child); >> + rle = resource_list_add(&devi->rl, type, rid, start, >> + start + count - 1, count); >> + if (rle == NULL) >> + return (ENXIO); >> + >> + return (0); >> +} > > Isn't this the same as bus_generic_rl_set_resource()? > >> + >> +static struct resource * >> +iicbus_alloc_resource(device_t bus, device_t child, int type, int *rid, >> + u_long start, u_long end, u_long count, u_int flags) >> +{ >> + struct resource_list *rl; >> + struct resource_list_entry *rle; >> + >> + /* Only IRQ resources are supported. */ >> + if (type != SYS_RES_IRQ) >> + return (NULL); >> + >> + /* >> + * Request for the default allocation with a given rid: use resource >> + * list stored in the local device info. >> + */ >> + if ((start == 0UL) && (end == ~0UL)) { >> + rl = BUS_GET_RESOURCE_LIST(bus, child); >> + if (rl == NULL) >> + return (NULL); >> + rle = resource_list_find(rl, type, *rid); >> + if (rle == NULL) { >> + if (bootverbose) >> + device_printf(bus, "no default resources for " >> + "rid = %d, type = %d\n", *rid, type); >> + return (NULL); >> + } >> + start = rle->start; >> + end = rle->end; >> + count = rle->count; >> + } >> + >> + return (bus_generic_alloc_resource(bus, child, type, rid, start, end, >> + count, flags)); > > If you are using a resource list, you should be using resource_list_alloc(). > However, I think you can replace this entire method with just > bus_generic_rl_alloc_resource(). > >> @@ -297,6 +366,16 @@ static device_method_t iicbus_methods[] >> DEVMETHOD(device_detach, iicbus_detach), >> >> /* bus interface */ >> + DEVMETHOD(bus_setup_intr, bus_generic_setup_intr), >> + DEVMETHOD(bus_teardown_intr, bus_generic_teardown_intr), >> + DEVMETHOD(bus_release_resource, bus_generic_release_resource), > > After fixing alloc_resource to use resource_list_alloc() (either directly > or via the generic method), this should be set to > bus_generic_rl_release_resource(). > >> Modified: head/sys/dev/ofw/ofw_iicbus.c >> ============================================================================== >> --- head/sys/dev/ofw/ofw_iicbus.c Sat May 9 00:48:44 2015 (r282673) >> +++ head/sys/dev/ofw/ofw_iicbus.c Sat May 9 03:05:44 2015 (r282674) >> @@ -199,3 +205,12 @@ ofw_iicbus_get_devinfo(device_t bus, dev >> dinfo = device_get_ivars(dev); >> return (&dinfo->opd_obdinfo); >> } >> + >> +static struct resource_list * >> +ofw_iicbus_get_resource_list(device_t bus __unused, device_t child) >> +{ >> + struct ofw_iicbus_devinfo *devi; >> + >> + devi = device_get_ivars(child); >> + return (&devi->opd_dinfo.rl); >> +} > > I think you don't actually need this since the inherited method should > already work. > > -- > John Baldwin Yeah, that's all correct, Michal had already warned about a few of these issues. Fixed in r282702. Thanks for the review. Luiz