From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julien Grall <julien.grall@linaro.org>
Subject: Re: [PATCH ARM v4 11/12] mini-os: get GIC addresses
 from FDT
Date: Thu, 19 Jun 2014 11:58:07 +0100
Message-ID: <53A2C23F.4050902@linaro.org>
References: <1403104106-32538-1-git-send-email-talex5@gmail.com>	<1403104106-32538-12-git-send-email-talex5@gmail.com>	<53A1CB6F.4050209@linaro.org>
	<CAG4opy_1Hi=0UrD+rnCdU9DjSXR6UgrOTOBLvifqF5SSbqP2-w@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta5.messagelabs.com ([195.245.231.135])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <julien.grall@linaro.org>) id 1Wxa2l-0001oM-BO
	for xen-devel@lists.xenproject.org; Thu, 19 Jun 2014 10:58:11 +0000
Received: by mail-we0-f181.google.com with SMTP id q59so2184227wes.12
	for <xen-devel@lists.xenproject.org>;
	Thu, 19 Jun 2014 03:58:10 -0700 (PDT)
In-Reply-To: <CAG4opy_1Hi=0UrD+rnCdU9DjSXR6UgrOTOBLvifqF5SSbqP2-w@mail.gmail.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Thomas Leonard <talex5@gmail.com>
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, David Scott <Dave.Scott@eu.citrix.com>, Samuel Thibault <samuel.thibault@ens-lyon.org>, Anil Madhavapeddy <anil@recoil.org>
List-Id: xen-devel@lists.xenproject.org

Hi Thomas,

On 06/19/2014 09:50 AM, Thomas Leonard wrote:
> On 18 June 2014 18:25, Julien Grall <julien.grall@linaro.org> wrote:
>> Hi Thomas,
>>
>> On 06/18/2014 04:08 PM, Thomas Leonard wrote:
>>>  //#define VGIC_DEBUG
>>>  #ifdef VGIC_DEBUG
>>> @@ -168,9 +169,38 @@ static void gic_handler(void) {
>>>  }
>>>
>>>  void gic_init(void) {
>>> -    // FIXME Get from dt!
>>> -    gic.gicd_base = (char *)0x2c001000ULL;
>>> -    gic.gicc_base = (char *)0x2c002000ULL;
>>> +    gic.gicd_base = NULL;
>>
>> Any reason to not fold this patch in patch #7? Or better move the gic
>> code in a separate patch?
> 
> It was previously requested that I split the FDT patch from the main
> ARM one. This patch depends on libfdt being present, so it has to go
> after that.

Please make sure that this "standalone patch" works correctly on Xen
unstable...

> Moving all the GIC code to this patch would mean that the original
> patch wouldn't work on its own.

I'm not sure why you want to have a single big patch to support ARM...
AFAIU, there is some patch requirements to work correctly.


>> You made assumption of the layout of the device tree provided by Xen:
>>         - #address-cells == #size-cells == 2
>>         - regs contains a valid physical address, i.e the device is not under a bus
>>
>> This can be changed by the toolstack in the future and will likely break
>> mini-os.
> 
> Is that likely? Seems like using BUG here is the right thing to do
> until that happens.

You need at least to add a comment about the (len != 32). It's confusing
for people that doesn't know how the device tree has been created.

I still think that a layer for IRQ and MMIO translation would be helpful
for mini-os. So you will directly get the range (base + size), and won't
duplicate checking code on every part of mini-os.

> That would be very helpful - thanks!

You can join #xenarm for any question if you are not already there.

Regards,

-- 
Julien Grall