lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 16 Jan 2015 14:09:26 +0000
From:	Mark Rutland <mark.rutland@....com>
To:	Orson Zhai <orsonzhai@...il.com>, marc.zyngier@....com
Cc:	Chunyan Zhang <chunyan.zhang@...eadtrum.com>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"arnd@...db.de" <arnd@...db.de>,
	"gnomes@...rguk.ukuu.org.uk" <gnomes@...rguk.ukuu.org.uk>,
	"broonie@...nel.org" <broonie@...nel.org>,
	"robh+dt@...nel.org" <robh+dt@...nel.org>,
	Pawel Moll <Pawel.Moll@....com>,
	"ijc+devicetree@...lion.org.uk" <ijc+devicetree@...lion.org.uk>,
	"galak@...eaurora.org" <galak@...eaurora.org>,
	Will Deacon <Will.Deacon@....com>,
	Catalin Marinas <Catalin.Marinas@....com>,
	"jslaby@...e.cz" <jslaby@...e.cz>,
	"jason@...edaemon.net" <jason@...edaemon.net>,
	"heiko@...ech.de" <heiko@...ech.de>,
	"florian.vaussard@...l.ch" <florian.vaussard@...l.ch>,
	"andrew@...n.ch" <andrew@...n.ch>,
	"rrichter@...ium.com" <rrichter@...ium.com>,
	"hytszk@...il.com" <hytszk@...il.com>,
	"grant.likely@...aro.org" <grant.likely@...aro.org>,
	"antonynpavlov@...il.com" <antonynpavlov@...il.com>,
	"Joel.Schopp@....com" <Joel.Schopp@....com>,
	"suravee.suthikulpanit@....com" <suravee.suthikulpanit@....com>,
	"shawn.guo@...aro.org" <shawn.guo@...aro.org>,
	"jorge.ramirez-ortiz@...aro.org" <jorge.ramirez-ortiz@...aro.org>,
	"lee.jones@...aro.org" <lee.jones@...aro.org>,
	"geng.ren@...eadtrum.com" <geng.ren@...eadtrum.com>,
	"zhizhou.zhang@...eadtrum.com" <zhizhou.zhang@...eadtrum.com>,
	"lanqing.liu@...eadtrum.com" <lanqing.liu@...eadtrum.com>,
	"zhang.lyra@...il.com" <zhang.lyra@...il.com>,
	"wei.qiao@...eadtrum.com" <wei.qiao@...eadtrum.com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Leo Yan <leo.yan@...aro.org>
Subject: Re: [PATCH v5 3/5] arm64: dts: Add support for Spreadtrum SC9836 SoC
 in dts and Makefile

On Fri, Jan 16, 2015 at 12:49:20PM +0000, Orson Zhai wrote:
> Hi, Mark,
> 
> On Fri, Jan 16, 2015 at 6:35 PM, Mark Rutland <mark.rutland@....com> wrote:
> > On Fri, Jan 16, 2015 at 10:00:09AM +0000, Chunyan Zhang wrote:
> >> From: Zhizhou Zhang <zhizhou.zhang@...eadtrum.com>
> >>
> >> Adds the device tree support for Spreadtrum SC9836 SoC which is based on
> >> Sharkl64 platform.
> >>
> >> Sharkl64 platform contains the common nodes of Spreadtrum's arm64-based SoCs.
> >>
> >> Signed-off-by: Zhizhou Zhang <zhizhou.zhang@...eadtrum.com>
> >> Signed-off-by: Orson Zhai <orson.zhai@...eadtrum.com>
> >> Signed-off-by: Chunyan Zhang <chunyan.zhang@...eadtrum.com>
> >> ---
> >>  arch/arm64/boot/dts/Makefile                  |    1 +
> >>  arch/arm64/boot/dts/sprd/Makefile             |    5 ++
> >>  arch/arm64/boot/dts/sprd/sc9836-openphone.dts |   49 +++++++++++++++++
> >>  arch/arm64/boot/dts/sprd/sc9836.dtsi          |   73 +++++++++++++++++++++++++
> >>  arch/arm64/boot/dts/sprd/sharkl64.dtsi        |   67 +++++++++++++++++++++++
> >>  5 files changed, 195 insertions(+)
> >>  create mode 100644 arch/arm64/boot/dts/sprd/Makefile
> >>  create mode 100644 arch/arm64/boot/dts/sprd/sc9836-openphone.dts
> >>  create mode 100644 arch/arm64/boot/dts/sprd/sc9836.dtsi
> >>  create mode 100644 arch/arm64/boot/dts/sprd/sharkl64.dtsi
> >
> > [...]
> >
> >> +     cpus {
> >> +             #address-cells = <2>;
> >> +             #size-cells = <0>;
> >> +
> >> +             cpu@0 {
> >> +                     device_type = "cpu";
> >> +                     compatible = "arm,cortex-a53", "arm,armv8";
> >> +                     reg = <0x0 0x0>;
> >> +                     enable-method = "psci";
> >> +             };
> >> +
> >> +             cpu@1 {
> >> +                     device_type = "cpu";
> >> +                     compatible = "arm,cortex-a53", "arm,armv8";
> >> +                     reg = <0x0 0x1>;
> >> +                     enable-method = "psci";
> >> +             };
> >> +
> >> +             cpu@2 {
> >> +                     device_type = "cpu";
> >> +                     compatible = "arm,cortex-a53", "arm,armv8";
> >> +                     reg = <0x0 0x2>;
> >> +                     enable-method = "psci";
> >> +             };
> >> +
> >> +             cpu@3 {
> >> +                     device_type = "cpu";
> >> +                     compatible = "arm,cortex-a53", "arm,armv8";
> >> +                     reg = <0x0 0x3>;
> >> +                     enable-method = "psci";
> >> +             };
> >> +     };
> >
> > Just to check, all CPUs may be hotplugged off and on, yes?
> >
> 
> Yes, I have tested with them successfully by looking into
> `/proc/interrupts` and `top` except CPU0.

Ok.

> > Including CPU0?
> >
> 
>  It returns "status busy" after I type the command below.

Is this while other CPUs are active, or after you've hotplugged out all
other CPUs? The latter is expected, but the former is not.

Are you able to hotplug CPU0 out and back in while other CPUs are
active

> > How is your implementation tested?
> >
> I build kernel image with 3.19-rc1 + this patchset and run into console.
> I use `echo 0 > /sys/devices/system/cpu[0-3]/online`
> 
> BTW, I run these on a real phone of sc9836 not fast-model as before.
> 
> > You boot CPUs at EL2?
> >
> 
> Yes, I have confirmed this when working around the BUG() in
> arch_counter_get_cntpct() introduced from 3.19.

Ah, good to hear.

> 
> >> +
> >> +     gic: interrupt-controller@...01000 {
> >> +             compatible = "arm,gic-400";
> >> +             #interrupt-cells = <3>;
> >> +             interrupt-controller;
> >> +             reg = <0 0x12001000 0 0x1000>,
> >> +                   <0 0x12002000 0 0x2000>,
> >> +                   <0 0x12004000 0 0x2000>,
> >> +                   <0 0x12006000 0 0x2000>;
> >> +     };
> >
> > You're missing the maintenance interrupt here.
> >
> 
> Do you mean to declare SGI like this ?
> 
> " interrupts = <1 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)> "

Something like that, yes. However the maintenance interrupt is wired up
in your system.

> 
> > [...]
> >
> >> diff --git a/arch/arm64/boot/dts/sprd/sharkl64.dtsi b/arch/arm64/boot/dts/sprd/sharkl64.dtsi
> >> new file mode 100644
> >> index 0000000..b08989d
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/sprd/sharkl64.dtsi
> >> @@ -0,0 +1,67 @@
> >> +/*
> >> + * Spreadtrum Sharkl64 platform DTS file
> >> + *
> >> + * Copyright (C) 2014, Spreadtrum Communications Inc.
> >> + *
> >> + * This file is licensed under a dual GPLv2 or X11 license.
> >> + */
> >> +
> >> +/ {
> >> +     interrupt-parent = <&gic>;
> >> +     #address-cells = <2>;
> >> +     #size-cells = <2>;
> >> +
> >> +     soc {
> >> +             compatible = "simple-bus";
> >> +             reg = <0x0 0x0 0x0 0x80000000>;
> >
> > What is this reg for? It's not required by simple-bus.
> >
> 
> It's added by me.
> I want to tell people the register range of what the bus covers, not
> for any drivers use.

My point is that it's not part of the binding, so shouldn't be there.

> for this example, It starts from address 0 to 0x80000000.
> Because Spreadtrum chip is very large with a lots of registers and matrix buses.

Ok.

> 
> > If you want to encode that this covers a particular portion of the
> > address space, do so with the ranges proeprty.
> 
> But I look up the ePAPER who says "The ranges property provides a
> means of defining a mapping or translation...".
> The bus here is flat-memory for all.

In this case you could have:

ranges = <0x0 0x0 0x0 0x0 0x0 0x80000000>;

Which would be a flat mapping for the range you care about.

> 
> >
> >> +             #address-cells = <2>;
> >> +             #size-cells = <2>;
> >> +             ranges;
> >> +
> >> +             ap_apb: apb@...00000 {
> >> +                     compatible = "simple-bus";
> >> +                     reg = <0x0 0x70000000 0x0 0x10000000>;

For this bus you could instead use addresses relative to the bus inside,
rather than absolute addresses, or you could have:

ranges = <0x0 0x70000000 0x0 0x70000000 0x0 0x10000000>;

> >
> > Likewise here.
> >
> 
> This initial patch is picked up from a very big dt file.
> There are several apb buses in this chip.

At the same level us the bus hierarchy?

> So I use apb@...rting-address to separate them.
> But I remember another rule that the @address needs to equal  first
> address in property reg array.
> Do I have to delete @7000000 as well if i delete reg line?

Hmm. I'm not too keen on encoding a reg or unit-address here, because
the control interface of the bus isn't at that address. If we want to
add that later then the reg would be different in those cases. Given
there's no control interface here, there shouldn't be a reg or
unit-address.

Just ensure that the name before the unit-address is unique.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ