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:	Thu, 12 Mar 2015 17:05:41 +0000
From:	Mark Rutland <mark.rutland@....com>
To:	Kumar Gala <galak@...eaurora.org>
Cc:	"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...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>,
	"arm@...nel.org" <arm@...nel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"heiko@...ech.de" <heiko@...ech.de>
Subject: Re: [PATCH v3 2/4] arm64: dts: Add Qualcomm MSM8916 SoC and
 evaluation board dts

Hi Kumar,

> +/ {
> +	model = "Qualcomm Technologies, Inc. MSM 8916 MTP";
> +	compatible = "qcom,msm8916-mtp", "qcom,msm8916-mtp-smb1360",
> +			"qcom,msm8916", "qcom,mtp";
> +};

No /chosen/stdout-path?

Does your UART driver support earlycon?

[...]

> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		CPU0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53", "arm,armv8";
> +			reg = <0x0>;
> +		};
> +
> +		CPU1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53", "arm,armv8";
> +			reg = <0x1>;
> +		};
> +
> +		CPU2: cpu@2 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53", "arm,armv8";
> +			reg = <0x2>;
> +		};
> +
> +		CPU3: cpu@3 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53", "arm,armv8";
> +			reg = <0x3>;
> +		};
> +	};

The secondary CPUs need an enable-method. Are you using PSCI or
spin-table? 

Which exception level do the CPUs enter the kernel?

> +	timer {
> +		compatible = "arm,armv7-timer";

This should be "arm,armv8-timer".

> +		interrupts = <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 3 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 4 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 1 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
> +		clock-frequency = <19200000>;
> +	};

NAK. CNTFRQ should be programmed on all CPUs prior to entering the
kernel, per the boot protocol. You should not need clock-frequency here.

[...]

> +		intc: interrupt-controller@...0000 {
> +			compatible = "qcom,msm-qgic2";

This string isn't documented (but seems to be supported by the GIC
driver).

How does this differ from other GIC implementations?

> +			interrupt-controller;
> +			#interrupt-cells = <3>;
> +			reg = <0x0b000000 0x1000>, <0x0b002000 0x1000>;
> +		};

No GICH, GICV, maintenance interrupt?

Minor nit, but I'd prefer if the reg entries were on individual lines as
happens in other dts.

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