[PATCH 2/2] ARM: dts: mvebu: add device tree for IIJ SA-W2 appliance

[INAGAKI Hiroshi]

Hi Andrew,

thank you for your reviews and detailed descriptions.

On 2023/02/23 23:43, Andrew Lunn wrote:
>> +		pcie {
>> +			status = "okay";
>> +
>> +			pcie at 1,0 {
>> +				status = "okay";
>> +
>> +				/* Atheros AR9287 */
>> +				wifi at 0,0 {
>> +					compatible = "pci168c,002e";
>> +					reg = <0000 0 0 0 0>;
>> +				};
>> +			};
>> +
>> +			pcie at 3,0 {
>> +				status = "okay";
>> +
>> +				/* Qualcomm Atheros QCA9880 */
>> +				wifi at 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 at 1 {
>> +		compatible = "usb424,2514";
>> +		reg = <1>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		hub_port1: port at 1 {
>> +			reg = <1>;
>> +			#trigger-source-cells = <0>;
>> +		};
>> +
>> +		hub_port2: port at 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