From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [RFC 03/19] xen/arm: follow-up to allow DOM0 manage IRQ and MMIO Date: Thu, 03 Jul 2014 12:23:29 +0100 Message-ID: <53B53D31.5030908@linaro.org> References: <1402935486-29136-1-git-send-email-julien.grall@linaro.org> <1402935486-29136-4-git-send-email-julien.grall@linaro.org> <53A1F763.1030102@linaro.org> <1404385377.14865.68.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1X2f70-0000FG-0q for xen-devel@lists.xenproject.org; Thu, 03 Jul 2014 11:23:34 +0000 Received: by mail-we0-f170.google.com with SMTP id w61so73805wes.1 for ; Thu, 03 Jul 2014 04:23:32 -0700 (PDT) In-Reply-To: <1404385377.14865.68.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel@lists.xenproject.org, tim@xen.org, stefano.stabellini@citrix.com, Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On 07/03/2014 12:02 PM, Ian Campbell wrote: > On Wed, 2014-06-18 at 21:32 +0100, Julien Grall wrote: >>>> @@ -865,10 +888,9 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo, >>>> * property. Therefore these device doesn't need to be mapped. This >>>> * solution can be use later for pass through. >>>> */ >>>> - if ( !dt_device_type_is_equal(node, "memory") && >>>> - dt_device_is_available(node) ) >>>> + if ( !dt_device_type_is_equal(node, "memory") ) >>>> { >>>> - res = map_device(d, node); >>>> + res = handle_device(d, node, dt_device_is_available(node)); >>>> >>>> if ( res ) >>>> return res; >>> >>> We need a comment here >> >> Hmmm... I don't see what kind of comment I can add here. There is >> already lots of comments explaining handle_device and the previous if. > > I'd be inclined to push the dt_device_is_available call down into the > handle_function. > > Otherwise I would go with > res = make_deevice_available(node) What will do make_device_available? > if (!res && dt_...available(node) > res = map_device(node). AFAIU, it would means to duplicate the loop to get interrupt/MMIO twice which I think it's stupid. So, I prefer to push the dt_device_is_available call down into the handle_function. Regards, -- Julien Grall