linux-kernel - Re: [PATCH 3/5] ARM: dts: add dts files for bcmbca soc 47622

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]

Message-ID: <147e23b6-c5a3-a9fa-c0e1-defe906e42a9@broadcom.com>
Date:   Tue, 12 Apr 2022 08:37:02 -0700
From:   William Zhang <william.zhang@...adcom.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Broadcom Kernel List <bcm-kernel-feedback-list@...adcom.com>
Cc:     Samyon Furman <samyon.furman@...adcom.com>,
        Joel Peshkin <joel.peshkin@...adcom.com>,
        Florian Fainelli <florian.fainelli@...adcom.com>,
        Dan Beygelman <dan.beygelman@...adcom.com>,
        Tomer Yacoby <tomer.yacoby@...adcom.com>,
        Anand Gore <anand.gore@...adcom.com>,
        Kursad Oney <kursad.oney@...adcom.com>,
        Arnd Bergmann <arnd@...db.de>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Olof Johansson <olof@...om.net>,
        Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        soc@...nel.org
Subject: Re: [PATCH 3/5] ARM: dts: add dts files for bcmbca soc 47622

Hi Krzysztof,

On 4/12/22 04:27, Krzysztof Kozlowski wrote:
> On 11/04/2022 19:28, William Zhang wrote:
>> Add dts for ARMv7 based broadband SoC BCM47622. bcm47622.dtsi is the
>> SoC description dts header and bcm947622.dts is a simple dts file for
>> Broadcom BCM947622 Reference board that only enable the UART port.
>>
> 
> 
> Thank you for your patch. There is something to discuss/improve.
> 
>> + */
>> +
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +/ {
>> +	compatible = "brcm,bcm47622";
> 
> This does not match your bindings. Even if it is not used.
Did I miss anything? But it matches one of the compatible strings in 
brcm,bcmbca.yaml.

> 
>> +	#address-cells = <1>;
>> +	#size-cells = <1>;
>> +
>> +	interrupt-parent = <&gic>;
>> +
>> +	cpus {
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		CA7_0: cpu@0 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a7";
>> +			reg = <0x0>;
>> +			next-level-cache = <&L2_0>;
>> +			enable-method = "psci";
>> +		};
>> +
>> +		CA7_1: cpu@1 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a7";
>> +			reg = <0x1>;
>> +			next-level-cache = <&L2_0>;
>> +			enable-method = "psci";
>> +		};
>> +		CA7_2: cpu@2 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a7";
>> +			reg = <0x2>;
>> +			next-level-cache = <&L2_0>;
>> +			enable-method = "psci";
>> +		};
>> +		CA7_3: cpu@3 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a7";
>> +			reg = <0x3>;
>> +			next-level-cache = <&L2_0>;
>> +			enable-method = "psci";
>> +		};
>> +		L2_0: l2-cache0 {
>> +			compatible = "cache";
>> +		};
>> +	};
>> +
>> +	timer {
>> +		compatible = "arm,armv7-timer";
>> +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +			<GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +			<GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +			<GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
>> +		arm,cpu-registers-not-fw-configured;
>> +	};
>> +
>> +	pmu: pmu {
>> +		compatible = "arm,cortex-a7-pmu";
>> +		interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
>> +			<GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
>> +			<GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
>> +			<GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
>> +		interrupt-affinity = <&CA7_0>, <&CA7_1>,
>> +			<&CA7_2>, <&CA7_3>;
>> +	};
>> +
>> +	clocks: clocks {
>> +		periph_clk: periph_clk {
> 
> No underscores in node names.
Will update

> 
>> +			compatible = "fixed-clock";
>> +			#clock-cells = <0>;
>> +			clock-frequency = <200000000>;
>> +		};
>> +		uart_clk: uart_clk {
>> +			compatible = "fixed-factor-clock";
>> +			#clock-cells = <0>;
>> +			clocks = <&periph_clk>;
>> +			clock-div = <4>;
>> +			clock-mult = <1>;
>> +		};
>> +	};
>> +
>> +	psci {
>> +		compatible = "arm,psci-0.2";
>> +		method = "smc";
>> +		cpu_off = <1>;
>> +		cpu_on = <2>;
>> +	};
>> +
>> +	axi@...00000 {
>> +		compatible = "simple-bus";
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		ranges = <0 0x81000000 0x818000>;
>> +
>> +		gic: interrupt-controller@...0 {
>> +			compatible = "arm,cortex-a7-gic";
>> +			#interrupt-cells = <3>;
>> +			#address-cells = <0>;
>> +			interrupt-controller;
>> +			reg = <0x1000 0x1000>,
>> +				<0x2000 0x2000>;
>> +		};
>> +	};
>> +
>> +	periph-bus@...00000 {
> 
> just "bus" to be generic?
This node represents the peripheral bus in the soc. Would prefer to use 
the specific name unless it is not allowed by the linux dts.

> 
> 
> 
> Best regards,
> Krzysztof

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4212 bytes)