Message-ID: <7d4a218d-8b8a-5a1d-eff8-e154bfde69be@gmail.com>
Date:   Fri, 24 Feb 2023 21:57:37 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH 2/2] ARM: dts: mvebu: add device tree for IIJ SA-W2
 appliance
Content-Language: en-US
To:     Andrew Lunn <andrew@lunn.ch>
Cc:     devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, gregory.clement@bootlin.com,
        sebastian.hesselbarth@gmail.com, arnd@arndb.de, olof@lixom.net,
        soc@kernel.org, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org
References: <20230223132502.2045-1-musashino.open@gmail.com>
 <20230223132502.2045-2-musashino.open@gmail.com> <Y/d7gjqQCKKXMHqj@lunn.ch>
From:   INAGAKI Hiroshi <musashino.open@gmail.com>
In-Reply-To: <Y/d7gjqQCKKXMHqj@lunn.ch>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Andrew,

thank you for your reviews and detailed descriptions.

On 2023/02/23 23:43, Andrew Lunn wrote:
>> +		pcie {
>> +			status = "okay";
>> +
>> +			pcie@1,0 {
>> +				status = "okay";
>> +
>> +				/* Atheros AR9287 */
>> +				wifi@0,0 {
>> +					compatible = "pci168c,002e";
>> +					reg = <0000 0 0 0 0>;
>> +				};
>> +			};
>> +
>> +			pcie@3,0 {
>> +				status = "okay";
>> +
>> +				/* Qualcomm Atheros QCA9880 */
>> +				wifi@0,0 {
>> +					compatible = "qcom,ath10k";
>> +					reg = <0000 0 0 0 0>;
>> +				};
>> +			};
>> +		};
>> +	};
> These are not wrong, but they are also not needed. PCI devices should
> be discovered by enumeration, and you don't have any additional
> properties here, or phandles pointing to these nodes.
>
> I assume these are COTS wifi modules? By listing them here you are
> restricting some flexibility. The OEM could for example swap the
> modules around, and Linux would not care, but the DT would then be
> wrong. Or you could have a device with a different module because it
> is cheaper, and again, Linux would not care, but the DT would be
> wrong.

Got it. SA-W2 is not designed to allow users to swap cards under 
normal use, but certainly things like you said can happen...
I'll remove "wifi" nodes.

 > I assume these are COTS wifi modules?

Yes, those are the modules manufactured by Silex Technology, Inc. [1][2].

[1]: https://www.silex.jp/products/wireless-module/sxpcegn.html
[2]: https://www.silex.jp/products/wireless-module/sxpceac.html

>
>> +&usb0 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pmx_usb_pins>;
>> +	status = "okay";
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +
>> +	/* SMSC USB2514B */
>> +	hub@1 {
>> +		compatible = "usb424,2514";
>> +		reg = <1>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		hub_port1: port@1 {
>> +			reg = <1>;
>> +			#trigger-source-cells = <0>;
>> +		};
>> +
>> +		hub_port2: port@2 {
>> +			reg = <2>;
>> +			#trigger-source-cells = <0>;
>> +		};
>> +	};
>> +};
> Same comment as PCI. However, it is likely that the USB hub is
> actually on the board, not a module, so it is a lot less likely to
> change.

Yes, that USB hub is on the PCB and wired to the SoC directly. But 
I'll keep it in mind...

>
> As i said, they are not wrong, so you don't need to remove them.
>
> 	Andrew
>

Regards,
Hiroshi