From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <Ian.Campbell@citrix.com>
Subject: Re: [PATCH 01/10] xen/arm: Implement hip04-d01 platform
Date: Tue, 13 Jan 2015 15:23:55 +0000
Message-ID: <1421162635.19103.110.camel@citrix.com>
References: <1415009522-6344-1-git-send-email-frediano.ziglio@huawei.com>
	<1415009522-6344-2-git-send-email-frediano.ziglio@huawei.com>
	<1421150283.19103.39.camel@citrix.com>
	<CAHt6W4finxqFLZSxb_BqJgaW0DeUcEk+D0yN_JKfgcBk-hW9hA@mail.gmail.com>
	<1421160174.19103.93.camel@citrix.com>
	<CAHt6W4cZeLRqpG04uVgazSn5W1wrWhHSaoDJ9ZiUEVFU9qTeCw@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>
In-Reply-To: <CAHt6W4cZeLRqpG04uVgazSn5W1wrWhHSaoDJ9ZiUEVFU9qTeCw@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: Frediano Ziglio <freddy77@gmail.com>
Cc: zoltan.kiss@huawei.com, Julien Grall <julien.grall@linaro.org>, Tim Deegan <tim@xen.org>, xen-devel@lists.xen.org, Frediano Ziglio <frediano.ziglio@huawei.com>, Stefano Stabellini <stefano.stabellini@citrix.com>
List-Id: xen-devel@lists.xenproject.org

On Tue, 2015-01-13 at 15:11 +0000, Frediano Ziglio wrote:
> 2015-01-13 14:42 GMT+00:00 Ian Campbell <Ian.Campbell@citrix.com>:
> > On Tue, 2015-01-13 at 14:09 +0000, Frediano Ziglio wrote:
> >> 2015-01-13 11:58 GMT+00:00 Ian Campbell <Ian.Campbell@citrix.com>:
> >> > On Mon, 2014-11-03 at 10:11 +0000, Frediano Ziglio wrote:
> >> >> Add this new platform to Xen.
> >> >> This platform require specific code to initialize CPUs.
> >> >
> >> > What is the "bootwrapper"? Are you running this on real silicon or on an
> >> > emulator? Can the platform be made to do PSCI instead?
> >> >
> >>
> >> Very real. It's actually on my desk and I'm not in Matrix :-)
> >
> > OK. The choice of bootwrapper as a name is a bit unfortunate, since it
> > is already used for something else, but oh well.
> >
> >> Has no PSCI support. Would be honestly very great. As we (as company)
> >> write the firmware could be technically doable. There is no plan. This
> >> piece of software is meant to bring the CPU from Secure mode to
> >> Unsecure Hypervisor mode before calling kernel/hypervisor code and
> >> provide supervisor calls.
> >
> > Sounds a lot like PSCI to me, except non-standard ;-)
> >
> >> >> +    np_fab = dt_find_compatible_node(NULL, NULL, "hisilicon,hip04-fabric");
> >> >
> >> > Please add a reference to the DT bindings document for these values.
> >> >
> >> > linux/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
> >> > seems related but doesn't talk about most of these fields.
> >> >
> >>
> >> There are documentation in the Linaro kernel, see
> >> https://git.linaro.org/kernel/linux-linaro-tracking.git/blob/HEAD:/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt.
> >> I hope it will be merged soon.
> >
> > Thanks, but this doesn't seem to cover many of the properties used by
> > the code you are adding, e.g. bootwrapper-{size,magic},
> > relocation-{entry,size} (in fact it suggests they are part of a
> > boot-method array).
> >
> > I get the feeling these might be legacy/deprecated. Perhaps we could get
> > away without supporting such things?
> >
> > Ian.
> >
> 
> No, this is the newest code.

So in this:
    if ( !dt_property_read_u32(np, "bootwrapper-phys",
                               &hip04_boot.bootwrapper_phys) ) {
[...read boot-method array...]
    }
    else
    {
[... read a bunch of separate properties...]
    }

Both sides of the if are used by non-legacy code? That's,...
unfortunate.

I note that only the if case, not the else case, is covered by the
hisilicon.txt bindings...

Ian.