linux-kernel - Re: [PATCH 4/4] arm64: dts: Add Broadcom North Star 2 support

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] [day] [month] [year] [list]

Message-ID: <55A57D2B.4010207@broadcom.com>
Date:	Tue, 14 Jul 2015 14:20:43 -0700
From:	Ray Jui <rjui@...adcom.com>
To:	Mark Rutland <mark.rutland@....com>
CC:	Catalin Marinas <Catalin.Marinas@....com>,
	Will Deacon <Will.Deacon@....com>,
	Arnd Bergmann <arnd@...db.de>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Hauke Mehrtens <hauke@...ke-m.de>,
	Jon Mason <jonmason@...adcom.com>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"bcm-kernel-feedback-list@...adcom.com" 
	<bcm-kernel-feedback-list@...adcom.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 4/4] arm64: dts: Add Broadcom North Star 2 support



On 7/14/2015 2:23 AM, Mark Rutland wrote:
> Hi,
> 
>> +/dts-v1/;
>> +
>> +#include "ns2.dtsi"
>> +
>> +/ {
>> +       model = "Broadcom NS2 SVK";
>> +       compatible = "brcm,ns2-svk", "brcm,ns2";
>> +
>> +       chosen {
>> +               bootargs = "console=ttyS0,115200n8 earlycon=uart8250,mmio32,0x66130000";
>> +       };
> 
> Please use stdout-path instead (you can use /aliases to make it
> simpler). It'll save a redundant description of the UART and will remove
> the dependency on Linux-specific naming of the UART.
> 
> [...]

Saw that Arnd also has the same comment. I will fix this.

> 
>> +/ {
>> +       compatible = "brcm,ns2";
>> +       interrupt-parent = <&gic>;
>> +       #address-cells = <2>;
>> +       #size-cells = <2>;
>> +
>> +       cpus {
>> +               #address-cells = <2>;
>> +               #size-cells = <0>;
>> +
>> +               cpu@0 {
>> +                       device_type = "cpu";
>> +                       compatible = "arm,cortex-a57", "arm,armv8";
>> +                       reg = <0 0>;
>> +                       enable-method = "spin-table";
>> +                       cpu-release-addr = <0 0x84b00000>;
>> +               };
>> +       };
> 
> Shouldn't the other CPUs be described?

Let me give that a try but there's a chance that I cannot enable other 3
cores until we have a more stable bootloader configuration.

> 
> Using spin-table for SMP is somewhat unfortunate, as it comes with a
> number of problems (e.g. unwoken secondaries spinning in the kernel). I
> would strongly advise using PSCI instead.
>

Yes I agree and I'm fully aware PSCI is the preferred way of bringing up
ARMv8 cores. Unfortunately this is currently out of my control as this
is done in ARM Trusted Firmware that was handled by a different team
within Broadcom and they have very tight schedule and will not have time
to add PSCI support in the near term.

The plan is to use spin-table for now and convert to PSCI when we have
the support for it in our ATF. That should happen within the next couple
months.


>> +
>> +       timer {
>> +               compatible = "arm,armv8-timer";
>> +               interrupts = <GIC_PPI 13 (GIC_CPU_MASK_RAW(0xff) |
>> +                             IRQ_TYPE_EDGE_RISING)>,
>> +                            <GIC_PPI 14 (GIC_CPU_MASK_RAW(0xff) |
>> +                             IRQ_TYPE_EDGE_RISING)>,
>> +                            <GIC_PPI 11 (GIC_CPU_MASK_RAW(0xff) |
>> +                             IRQ_TYPE_EDGE_RISING)>,
>> +                            <GIC_PPI 10 (GIC_CPU_MASK_RAW(0xff) |
>> +                             IRQ_TYPE_EDGE_RISING)>;
>> +               clock-frequency = <25000000>;
>> +       };
> 
> Please fix your firmware to configure CNTFRQ_EL0, it is simply a bug not
> to, and using clock-frequency does not fix all the problems that not
> configuring it causes.

Yes will do this.

> 
> Thanks,
> Mark.
> 

Thanks,

Ray
--
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/