Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <ab529968-7dc5-f4c9-5ce4-5dc95b64838f@gmail.com>
Date:   Tue, 12 Apr 2022 19:49:55 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:96.0) Gecko/20100101
 Thunderbird/96.0
Subject: Re: [PATCH] ARM: dts: BCM5301X: Add DT for Buffalo WZR-1166DHP2
To:     SHIMAMOTO Takayoshi <takayoshi.shimamoto.360@gmail.com>
Cc:     Florian Fainelli <f.fainelli@gmail.com>,
        Hauke Mehrtens <hauke@hauke-m.de>,
        bcm-kernel-feedback-list@broadcom.com,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzk+dt@kernel.org>,
        linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20220412154432.12124-1-takayoshi.shimamoto.360@gmail.com>
From:   =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= <zajec5@gmail.com>
In-Reply-To: <20220412154432.12124-1-takayoshi.shimamoto.360@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 12.04.2022 17:44, SHIMAMOTO Takayoshi wrote:
> Buffalo WZR-1166DHP2  wireless router with
> 
>   - BCM4708A0
>   - 256MiB DDR2 SDRAM
>   - 128MiB NAND flash
>   - 2T2R 11ac/a/b/g/n Wi-Fi
>   - 4x 10/100M ethernet switch

Isn't that 10/100/1000?


> The hardware configuration is very similar to the WZR-1750DHP,
> except for memory size and number of antennas.
> 
> Signed-off-by: SHIMAMOTO Takayoshi <takayoshi.shimamoto.360@gmail.com>
> ---
>   .../boot/dts/bcm4708-buffalo-wzr-1166dhp2.dts | 189 ++++++++++++++++++
>   1 file changed, 189 insertions(+)
>   create mode 100644 arch/arm/boot/dts/bcm4708-buffalo-wzr-1166dhp2.dts
> 
> diff --git a/arch/arm/boot/dts/bcm4708-buffalo-wzr-1166dhp2.dts b/arch/arm/boot/dts/bcm4708-buffalo-wzr-1166dhp2.dts
> new file mode 100644
> index 000000000000..22e54a0a202a
> --- /dev/null
> +++ b/arch/arm/boot/dts/bcm4708-buffalo-wzr-1166dhp2.dts
> @@ -0,0 +1,189 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Broadcom BCM470X / BCM5301X ARM platform code.
> + * DTS for Buffalo WZR-1166DHP2
> + *
> + * Copyright (C) 2014 Rafał Miłecki <zajec5@gmail.com>
> + * Copyright (C) 2022 SHIMAMOTO Takayoshi <takayoshi.shimamoto.360@gmail.com>
> + */
> +
> +/dts-v1/;
> +
> +#include "bcm4708.dtsi"
> +#include "bcm5301x-nand-cs0-bch8.dtsi"
> +
> +/ {
> +	compatible = "buffalo,wzr-1166dhp2", "brcm,bcm4708";
> +	model = "Buffalo WZR-1166DHP2 (BCM4708)";

We stopped adding SoC name to the "model" value, please drop it.


> +	chosen {
> +		bootargs = "console=ttyS0,115200";
> +	};
> +
> +	memory@0 {
> +		device_type = "memory";
> +		reg = <0x00000000 0x08000000>,
> +		      <0x88000000 0x08000000>;
> +	};
> +
> +	spi {
> +		compatible = "spi-gpio";
> +		num-chipselects = <1>;
> +		gpio-sck = <&chipcommon 7 0>;
> +		gpio-mosi = <&chipcommon 4 0>;
> +		cs-gpios = <&chipcommon 6 0>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		hc595: gpio_spi@0 {
> +			compatible = "fairchild,74hc595";
> +			reg = <0>;
> +			registers-number = <1>;
> +			spi-max-frequency = <100000>;
> +
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +
> +		};
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		usb {
> +			label = "bcm53xx:blue:usb";
> +			gpios = <&hc595 0 GPIO_ACTIVE_HIGH>;
> +			trigger-sources = <&ohci_port1>, <&ehci_port1>,
> +					  <&xhci_port1>, <&ohci_port2>,
> +					  <&ehci_port2>;
> +			linux,default-trigger = "usbport";
> +		};
> +
> +		power0 {
> +			label = "bcm53xx:red:power";
> +			gpios = <&hc595 1 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		power1 {
> +			label = "bcm53xx:white:power";
> +			gpios = <&hc595 2 GPIO_ACTIVE_HIGH>;
> +			linux,default-trigger = "default-on";
> +		};
> +
> +		router0 {
> +			label = "bcm53xx:blue:router";
> +			gpios = <&hc595 3 GPIO_ACTIVE_HIGH>;
> +			linux,default-trigger = "default-on";
> +		};
> +
> +		router1 {
> +			label = "bcm53xx:amber:router";
> +			gpios = <&hc595 4 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		wan {
> +			label = "bcm53xx:blue:wan";
> +			gpios = <&hc595 5 GPIO_ACTIVE_HIGH>;
> +			linux,default-trigger = "default-on";
> +		};
> +
> +		wireless0 {
> +			label = "bcm53xx:blue:wireless";
> +			gpios = <&hc595 6 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		wireless1 {
> +			label = "bcm53xx:amber:wireless";
> +			gpios = <&hc595 7 GPIO_ACTIVE_HIGH>;
> +		};
> +	};

I'm wondering if all new DTS files should use "function" and "color"
instead of "label". That's preferred solution and I guess it's good idea
to use it for new code.